From c9810261956faa1152ade6a37efe6583b2caaa20 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 6 Sep 2023 11:12:23 +0200 Subject: [PATCH] extend comments around PassMode::Direct --- compiler/rustc_codegen_llvm/src/abi.rs | 42 ++++++++++++++++++++++- compiler/rustc_target/src/abi/call/mod.rs | 4 ++- compiler/rustc_ty_utils/src/abi.rs | 2 ++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 863cb7068f8..1e72b7db1e4 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -340,15 +340,53 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { }; for arg in args { + // Note that the exact number of arguments pushed here is carefully synchronized with + // code all over the place, both in the codegen_llvm and codegen_ssa crates. That's how + // other code then knows which LLVM argument(s) correspond to the n-th Rust argument. let llarg_ty = match &arg.mode { PassMode::Ignore => continue, - PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx), + PassMode::Direct(_) => { + // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges), + // and for Scalar ABIs the LLVM type is fully determined by `layout.abi`, + // guarnateeing that we generate ABI-compatible LLVM IR. Things get tricky for + // aggregates... + if matches!(arg.layout.abi, abi::Abi::Aggregate { .. }) { + // This is the most critical case for ABI compatibility, since + // `immediate_llvm_type` will use `layout.fields` to turn this Rust type + // into an LLVM type. ABI-compatible Rust types can have different `fields`, + // so we need to be very sure that LLVM wil treat those different types in + // an ABI-compatible way. Mostly we do this by disallowing + // `PassMode::Direct` for aggregates, but we actually do use that mode on + // wasm. wasm doesn't have aggregate types so we are fairly sure that LLVM + // will treat `{ i32, i32, i32 }` and `{ { i32, i32, i32 } }` the same way + // for ABI purposes. + assert!( + matches!(&*cx.tcx.sess.target.arch, "wasm32" | "wasm64"), + "`PassMode::Direct` for aggregates only allowed on wasm targets\nProblematic type: {:#?}", + arg.layout, + ); + } + arg.layout.immediate_llvm_type(cx) + } PassMode::Pair(..) => { + // ABI-compatible Rust types have the same `layout.abi` (up to validity ranges), + // so for ScalarPair we can easily be sure that we are generating ABI-compatible + // LLVM IR. + assert!( + matches!(arg.layout.abi, abi::Abi::ScalarPair(..)), + "PassMode::Pair for type {}", + arg.layout.ty + ); llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true)); llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 1, true)); continue; } PassMode::Indirect { attrs: _, extra_attrs: Some(_), on_stack: _ } => { + assert!(arg.layout.is_unsized()); + // Construct the type of a (wide) pointer to `ty`, and pass its two fields. + // Any two ABI-compatible unsized types have the same metadata type and + // moreover the same metadata value leads to the same dynamic size and + // alignment, so this respects ABI compatibility. let ptr_ty = Ty::new_mut_ptr(cx.tcx, arg.layout.ty); let ptr_layout = cx.layout_of(ptr_ty); llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true)); @@ -360,6 +398,8 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { if *pad_i32 { llargument_tys.push(Reg::i32().llvm_type(cx)); } + // Compute the LLVM type we use for this function from the cast type. + // We assume here that ABI-compatible Rust types have the same cast type. cast.llvm_type(cx) } PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => cx.type_ptr(), diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 8d573def9bb..9a905dbc806 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -36,7 +36,7 @@ pub enum PassMode { Ignore, /// Pass the argument directly. /// - /// The argument has a layout abi of `Scalar`, `Vector` or in rare cases `Aggregate`. + /// The argument has a layout abi of `Scalar`, `Vector` or in rare cases (e.g. on wasm) `Aggregate`. Direct(ArgAttributes), /// Pass a pair's elements directly in two arguments. /// @@ -465,6 +465,7 @@ pub struct ArgAbi<'a, Ty> { } impl<'a, Ty> ArgAbi<'a, Ty> { + /// This defines the "default ABI" for that type, that is then later adjusted in `fn_abi_adjust_for_abi`. pub fn new( cx: &impl HasDataLayout, layout: TyAndLayout<'a, Ty>, @@ -478,6 +479,7 @@ impl<'a, Ty> ArgAbi<'a, Ty> { scalar_attrs(&layout, b, a.size(cx).align_to(b.align(cx).abi)), ), Abi::Vector { .. } => PassMode::Direct(ArgAttributes::new()), + // The `Aggregate` ABI is almost always adjusted later. Abi::Aggregate { .. } => PassMode::Direct(ArgAttributes::new()), }; ArgAbi { layout, mode } diff --git a/compiler/rustc_ty_utils/src/abi.rs b/compiler/rustc_ty_utils/src/abi.rs index 802391f1aad..3ffe670d87a 100644 --- a/compiler/rustc_ty_utils/src/abi.rs +++ b/compiler/rustc_ty_utils/src/abi.rs @@ -520,6 +520,8 @@ fn fn_abi_adjust_for_abi<'tcx>( _ => return, } + // `Aggregate` ABI must be adjusted to ensure that ABI-compatible Rust types are passed + // the same way. let size = arg.layout.size; if arg.layout.is_unsized() || size > Pointer(AddressSpace::DATA).size(cx) {