use 'unadjusted' ABI for wasm LLVM intrinsics

[RalfJung]

See rust-lang/rust#139809 for context.

Currently this crate uses a mix of "C" and "unadjusted", sometimes even for the same target -- no idea why. I would assume it should be "unadjusted" everywhere when importing an LLVM intrinsic? I don't want to make any sweeping changes in this PR though so this only touches wasm.

Cc @alexcrichton

[rustbot]

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[bjorn3]

If anything I would like to get rid of the unadjusted ABI (which is equivalent to the broken abi for wasm). It isn't a real ABI. It is just whatever way cg_llvm decides to lower rust types to LLVM types, which frequently doesn't have any relation whatsoever with the rust type, especially for unions and enums. And even for regular structs it contains fake fields for padding. I would like to change cg_llvm eventually to only support lowering ArgAbi, not raw rust types. https://github.com/rust-lang/rust/blob/49e5e4e3a5610c240a717cb99003a5d5d3356679/compiler/rustc_codegen_llvm/src/type_of.rs#L15-L140 and https://github.com/rust-lang/rust/blob/49e5e4e3a5610c240a717cb99003a5d5d3356679/compiler/rustc_codegen_llvm/src/type_of.rs#L188-L250 should not be necessary when the unadjusted ABI is gone.

[RalfJung]

For calling LLVM intrinsics, I think this makes perfect sense. These are not real fn calls, so we don't need a real ABI.

[bjorn3]

LLVM intrinsic calls are still going through the regular ABI handling. If LLVM intrinsic calls are split out from the regular ABI handling and the LLVM intrinsic call handling is changed to only allow the unadjusted ABI (and the regular ABI handling to not allow unadjusted calls at all) and the unadjusted ABI stops supporting enums and unions, I did be fine with keeping unadjusted calls. The problem is that they are currently complicating regular calls a fair bit and thus also infecting other codegen backends that aren't LLVM based at all.

[Amanieu]

I read rust-lang/rust#139809 but don't see the relation with this PR. The issue that rust-lang/rust#139809 addresses is related to the public std::arch wrapper functions while this PR only touches the declarations of the private LLVM intrinsics.

Currently this crate uses a mix of "C" and "unadjusted", sometimes even for the same target -- no idea why.

Historical reasons: previously everything was using extern "C" and then extern "unadjusted" was added to address some specific issues (notably to pass i128 directly where it would be split into i64 halves on some targets). In this case it seems extern "C" is behaving correctly? Though I wouldn't be opposed to just switching everything to unadjusted.

[alexcrichton]

I believe the relation of rust-lang/rust#139809 and this PR is that the source of the warning reported here is a monomorphization-time (inlining-time?) warning about usage of std::arch. The std::arch module uses extern "C" (as is seen here) under the hood and the warning about C ABI transitions fires. This warning is a false positive, though, and usage of std::arch shouldn't trigger a warning.

[RalfJung]

LLVM intrinsic calls are still going through the regular ABI handling.

Yes, that's why we need "unadjusted" -- to disable the regular ABI handling for these intrinsics.

This could potentially be done more cleanly by doing this automatically for all LLVM intrinsics or so. But I am trying to fix an existing issue here, not redo the underlying system in a clean way.

I read rust-lang/rust#139809 but don't see the relation with this PR. The issue that rust-lang/rust#139809 addresses is related to the public std::arch wrapper functions while this PR only touches the declarations of the private LLVM intrinsics.

As Alex explained, this is a monomorphization-time lint, so the details of the private intrinsics can affect warnings shown in user crates.

[RalfJung]

In this case it seems extern "C" is behaving correctly?

On wasm-u-u, extern "C" is entirely unadjusted -- that's a bug we're trying to fix, see rust-lang/rust#138762.

[RalfJung]

the unadjusted ABI stops supporting enums and unions

I have no objections to restricting that ABI o just what stdarch needs. Scalars and SIMD vectors should probably be enough.

[bjorn3]

LLVM intrinsic calls are still going through the regular ABI handling.

Yes, that's why we need "unadjusted" -- to disable the regular ABI handling for these intrinsics.

extern "unadjusted" goes through the regular ABI handling in the backend. It just forces PassMode::Direct for all arguments even for those with types for which PassMode::Direct is completely nonsensical, thus forcing the regular ABI handling to make up an ABI for those types.

the unadjusted ABI stops supporting enums and unions

I have no objections to restricting that ABI o just what stdarch needs. Scalars and SIMD vectors should probably be enough.

stdarch also needs structs for some cases. It doesn't need structs with padding though afaik.

[RalfJung]

extern "unadjusted" goes through the regular ABI handling in the backend. It just forces PassMode::Direct for all arguments even for those with types for which PassMode::Direct is completely nonsensical, thus forcing the regular ABI handling to make up an ABI for those types.

I am fully aware of what "unadjusted" does. :) It disables regular ABI handling. You seem to make a difference between "disable regular ABI handling" and "go through regular ABI handling but make it a NOP (i.e., disabling all ABI adjustments)", I don't understand the point of that. We have to set some PassMode for all arguments, we can't just not have a PassMode, so setting it to Direct is how one disables ABI handling and leaves it to LLVM -- which is exact what we want for calling an LLVM intrinsic whose signature is defined in terms of LLVM types, not Rust types or C types.

stdarch also needs structs for some cases. It doesn't need structs with padding though afaik.

Ah, that is unfortunate. Yes that will leak some implementation details. It's highly unstable though and it's unclear to me what a better alternative would look like -- we want to call an LLVM intrinsic whose signature is defines in terms of LLVM types, this fundamentally has to rely on how Rust types are mapped to LLVM types.

[bjorn3]

What I want is in the implementation of the Call terminator before even creating an FnAbi have cg_llvm or cg_ssa catch all LLVM intrinsics and do completely separate lowering of the call to LLVM IR implementing exactly the subset of rust type -> LLVM type lowering that is needed without any regards for the FnAbi. That way the regular FnAbi handling never sees LLVM intrinsics at all.

[RalfJung]

Ah I see. Yeah I guess that could work, no strong opinion either way.