Rollup merge of rust-lang#157127 - scottmcm:tweak-layout-of-alternative, r=nikic

cg_LLVM: Stop needing an alloca for volatile loads

This ended up also being reimplementing it to not use `load` of the `llvm_type`, since without doing that everything blew up horribly.  cc the zulip conversation [#t-opsem > Defining volatile splitting @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Defining.20volatile.20splitting/near/597451615).

And while I'm here, improve the tests to check that the unaligned ones are actually unaligned, since `unaligned_volatile_load::<u8>` doesn't actually test anything.

r? @nikic
cc @RalfJung
MCP tracking issue rust-lang#153250

Author: JonathanBrouwer

compiler/rustc_codegen_gcc/src/builder.rs

@@ -993,7 +993,8 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
         loaded_value.to_rvalue()
     }
 
-    fn volatile_load(&mut self, ty: Type<'gcc>, ptr: RValue<'gcc>) -> RValue<'gcc> {
+    fn volatile_load(&mut self, ty: Type<'gcc>, ptr: RValue<'gcc>, _: Align) -> RValue<'gcc> {
+        // FIXME(antoyo): set alignment.
         let ptr = self.context.new_cast(self.location, ptr, ty.make_volatile().make_pointer());
         // (FractalFir): We insert a local here, to ensure this volatile load can't move across
         // blocks.

compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

@@ -5,7 +5,7 @@ mod simd;
 use std::iter;
 
 use gccjit::{ComparisonOp, Function, FunctionType, RValue, ToRValue, Type, UnaryOp};
-use rustc_abi::{BackendRepr, HasDataLayout, WrappingRange};
+use rustc_abi::{Align, BackendRepr, HasDataLayout, WrappingRange};
 use rustc_codegen_ssa::base::wants_msvc_seh;
 use rustc_codegen_ssa::common::IntPredicate;
 use rustc_codegen_ssa::errors::InvalidMonomorphization;
@@ -368,8 +368,9 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
 
             sym::volatile_load | sym::unaligned_volatile_load => {
                 let ptr = args[0].immediate();
-                let load = self.volatile_load(result.layout.gcc_type(self), ptr);
-                // FIXME(antoyo): set alignment.
+                let abi_align = result_layout.align.abi;
+                let ptr_align = if name == sym::volatile_load { abi_align } else { Align::ONE };
+                let load = self.volatile_load(result.layout.gcc_type(self), ptr, ptr_align);
                 if let BackendRepr::Scalar(scalar) = result.layout.backend_repr {
                     self.to_immediate_scalar(load, scalar)
                 } else {

compiler/rustc_codegen_llvm/src/builder.rs

@@ -682,9 +682,9 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         }
     }
 
-    fn volatile_load(&mut self, ty: &'ll Type, ptr: &'ll Value) -> &'ll Value {
+    fn volatile_load(&mut self, ty: &'ll Type, ptr: &'ll Value, align: Align) -> &'ll Value {
         unsafe {
-            let load = llvm::LLVMBuildLoad2(self.llbuilder, ty, ptr, UNNAMED);
+            let load = self.load(ty, ptr, align);
             llvm::LLVMSetVolatile(load, llvm::TRUE);
             load
         }

compiler/rustc_codegen_llvm/src/context.rs

@@ -978,7 +978,7 @@ impl<'ll, 'tcx> MiscCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
     }
 
     fn intrinsic_call_expects_place_always(&self, name: Symbol) -> bool {
-        matches!(name, sym::volatile_load | sym::unaligned_volatile_load | sym::black_box)
+        matches!(name, sym::black_box)
     }
 }
 

compiler/rustc_codegen_llvm/src/debuginfo/gdb.rs

@@ -1,5 +1,6 @@
 // .debug_gdb_scripts binary section.
 
+use rustc_abi::Align;
 use rustc_codegen_ssa::base::collect_debugger_visualizers_transitive;
 use rustc_codegen_ssa::traits::*;
 use rustc_hir::attrs::DebuggerVisualizerType;
@@ -18,10 +19,7 @@ pub(crate) fn insert_reference_to_gdb_debug_scripts_section_global(bx: &mut Buil
         let gdb_debug_scripts_section = get_or_insert_gdb_debug_scripts_section_global(bx);
         // Load just the first byte as that's all that's necessary to force
         // LLVM to keep around the reference to the global.
-        let volatile_load_instruction = bx.volatile_load(bx.type_i8(), gdb_debug_scripts_section);
-        unsafe {
-            llvm::LLVMSetAlignment(volatile_load_instruction, 1);
-        }
+        bx.volatile_load(bx.type_i8(), gdb_debug_scripts_section, Align::ONE);
     }
 }
 

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -354,25 +354,39 @@ impl<'ll, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
             }
 
             sym::volatile_load | sym::unaligned_volatile_load => {
-                let result = PlaceRef {
-                    val: result_place.unwrap(),
-                    layout: result_layout,
-                };
-
+                // Note that we cannot just load the `llvm_type` because we should never load non-scalars.
+                // Trying to do so blows up horribly in some cases -- for example loading a
+                // `MaybeUninint<&dyn Trait>` would load as `{ [i64x2] }` which gives assertions later
+                // (if we're lucky) from things not being pointers that ought to be.
                 let ptr = args[0].immediate();
-                let load = self.volatile_load(result_layout.llvm_type(self), ptr);
-                let align = if name == sym::unaligned_volatile_load {
-                    1
+                let abi_align = result_layout.align.abi;
+                let ptr_align = if name == sym::volatile_load { abi_align } else { Align::ONE };
+                if result_layout.is_zst() {
+                    return IntrinsicResult::Operand(OperandValue::ZeroSized);
+                } else if let BackendRepr::Scalar(scalar) = result_layout.backend_repr {
+                    let load = self.volatile_load(self.type_from_scalar(scalar), ptr, ptr_align);
+                    self.to_immediate_scalar(load, scalar)
                 } else {
-                    result_layout.align.bytes() as u32
-                };
-                unsafe {
-                    llvm::LLVMSetAlignment(load, align);
-                }
-                if !result_layout.is_zst() {
-                    self.store_to_place(load, result.val);
+                    // One day Rust will probably want to define how we split up a volatile load
+                    // of something that's *not* just an ordinary scalar, but for now we can just
+                    // use an LLVM integer type of the correct width and let it split it however.
+                    let llty = self.type_ix(result_layout.size.bits());
+                    let temp = if let Some(result_place) = result_place {
+                        PlaceRef {
+                            val: result_place,
+                            layout: result_layout,
+                        }
+                    } else {
+                        PlaceRef::alloca(self, result_layout)
+                    };
+                    let llval = self.volatile_load(llty, ptr, ptr_align);
+                    self.store(llval, temp.val.llval, abi_align);
+                    return if result_place.is_none() {
+                        IntrinsicResult::Operand(self.load_operand(temp).val)
+                    } else {
+                        IntrinsicResult::WroteIntoPlace
+                    };
                 }
-                return IntrinsicResult::WroteIntoPlace;
             }
             sym::volatile_store => {
                 let dst = args[0].deref(self.cx());

compiler/rustc_codegen_ssa/src/traits/builder.rs

@@ -242,7 +242,7 @@ pub trait BuilderMethods<'a, 'tcx>:
     fn alloca_with_ty(&mut self, layout: TyAndLayout<'tcx>) -> Self::Value;
 
     fn load(&mut self, ty: Self::Type, ptr: Self::Value, align: Align) -> Self::Value;
-    fn volatile_load(&mut self, ty: Self::Type, ptr: Self::Value) -> Self::Value;
+    fn volatile_load(&mut self, ty: Self::Type, ptr: Self::Value, align: Align) -> Self::Value;
     fn atomic_load(
         &mut self,
         ty: Self::Type,

tests/codegen-llvm/i128-x86-align.rs

@@ -5,8 +5,6 @@
 // while rustc wants it to have 16 byte alignment. This test checks that we handle this
 // correctly.
 
-// CHECK: %ScalarPair = type { i32, [3 x i32], i128 }
-
 #![feature(core_intrinsics)]
 
 #[repr(C)]
@@ -62,8 +60,8 @@ pub fn load_volatile(x: &ScalarPair) -> ScalarPair {
     // CHECK-SAME: dereferenceable(32) %_0,
     // CHECK-SAME: align 16
     // CHECK-SAME: dereferenceable(32) %x
-    // CHECK:      [[LOAD:%.*]] = load volatile %ScalarPair, ptr %x, align 16
-    // CHECK-NEXT: store %ScalarPair [[LOAD]], ptr %_0, align 16
+    // CHECK:      [[LOAD:%.*]] = load volatile i256, ptr %x, align 16
+    // CHECK-NEXT: store i256 [[LOAD]], ptr %_0, align 16
     // CHECK-NEXT: ret void
     unsafe { std::intrinsics::volatile_load(x) }
 }

tests/codegen-llvm/intrinsics/volatile.rs

@@ -5,6 +5,11 @@
 
 use std::intrinsics;
 
+#[repr(align(32))]
+pub struct CustomZst;
+
+type UninitFatPointer = std::mem::MaybeUninit<&'static dyn std::fmt::Debug>;
+
 // CHECK-LABEL: @volatile_copy_memory
 #[no_mangle]
 pub unsafe fn volatile_copy_memory(a: *mut u8, b: *const u8) {
@@ -28,8 +33,62 @@ pub unsafe fn volatile_set_memory(a: *mut u8, b: u8) {
 
 // CHECK-LABEL: @volatile_load
 #[no_mangle]
-pub unsafe fn volatile_load(a: *const u8) -> u8 {
-    // CHECK: load volatile
+pub unsafe fn volatile_load(a: *const u16) -> u16 {
+    // CHECK: [[TEMP:%.+]] = load volatile i16, ptr %a
+    // CHECK-SAME: align 2{{,|$}}
+    // CHECK-NEXT: ret i16 [[TEMP]]
+    intrinsics::volatile_load(a)
+}
+
+// CHECK-LABEL: @volatile_load_bool
+#[no_mangle]
+pub unsafe fn volatile_load_bool(a: *const bool) -> bool {
+    // CHECK: [[TEMP:%.+]] = load volatile i8, ptr %a
+    // CHECK-SAME: align 1{{,|$}}
+    // CHECK: [[TRUNC:%.+]] = trunc nuw i8 [[TEMP]] to i1
+    // CHECK: ret i1 [[TRUNC]]
+    intrinsics::volatile_load(a)
+}
+
+// CHECK-LABEL: @volatile_load_zst
+#[no_mangle]
+pub unsafe fn volatile_load_zst(a: *const CustomZst) -> CustomZst {
+    // CHECK: start:
+    // CHECK-NEXT: ret void
+    intrinsics::volatile_load(a)
+}
+
+// CHECK-LABEL: @volatile_load_array
+// CHECK-SAME: ptr{{.+}}sret([16 x i8]){{.+}}%_0
+#[no_mangle]
+pub unsafe fn volatile_load_array(a: *const [u16; 8]) -> [u16; 8] {
+    // CHECK-NOT: alloca
+    // CHECK: [[TEMP:%.+]] = load volatile i128, ptr %a,
+    // CHECK-SAME: align 2{{,|$}}
+    // CHECK: store i128 [[TEMP]], ptr %_0,
+    // CHECK-SAME: align 2{{,|$}}
+    // CHECK-NEXT: ret void
+    intrinsics::volatile_load(a)
+}
+
+// CHECK-LABEL: @volatile_load_fat
+#[no_mangle]
+pub unsafe fn volatile_load_fat(a: *const UninitFatPointer) -> UninitFatPointer {
+    // CHECK: [[ALLOCA:%.+]] = alloca
+    // CHECK-SAME: [[SIZE:4|8|16]] x i8
+    // CHECK-SAME: align [[ALIGN:2|4|8]]
+
+    // CHECK: [[TEMP:%.+]] = load volatile [[INT:i32|i64|i128]], ptr %a,
+    // CHECK-SAME: align [[ALIGN]]{{,|$}}
+    // CHECK: store [[INT]] [[TEMP]], ptr [[ALLOCA]],
+    // CHECK-SAME: align [[ALIGN]]{{,|$}}
+
+    // CHECK: [[T0:%.+]] = load ptr, ptr [[ALLOCA]], align [[ALIGN]]
+    // CHECK: [[T1:%.+]] = getelementptr inbounds i8, ptr [[ALLOCA]]
+    // CHECK: [[T2:%.+]] = load ptr, ptr [[T1]], align [[ALIGN]]
+    // CHECK: [[P1:%.+]] = insertvalue { ptr, ptr } poison, ptr [[T0]], 0
+    // CHECK: [[P2:%.+]] = insertvalue { ptr, ptr } [[P1]], ptr [[T2]], 1
+    // CHECK: ret { ptr, ptr } [[P2]]
     intrinsics::volatile_load(a)
 }
 
@@ -42,8 +101,62 @@ pub unsafe fn volatile_store(a: *mut u8, b: u8) {
 
 // CHECK-LABEL: @unaligned_volatile_load
 #[no_mangle]
-pub unsafe fn unaligned_volatile_load(a: *const u8) -> u8 {
-    // CHECK: load volatile
+pub unsafe fn unaligned_volatile_load(a: *const u16) -> u16 {
+    // CHECK: [[TEMP:%.+]] = load volatile i16, ptr %a
+    // CHECK-SAME: align 1{{,|$}}
+    // CHECK-NEXT: ret i16 [[TEMP]]
+    intrinsics::unaligned_volatile_load(a)
+}
+
+// CHECK-LABEL: @unaligned_volatile_load_bool
+#[no_mangle]
+pub unsafe fn unaligned_volatile_load_bool(a: *const bool) -> bool {
+    // CHECK: [[TEMP:%.+]] = load volatile i8, ptr %a
+    // CHECK-SAME: align 1{{,|$}}
+    // CHECK: [[TRUNC:%.+]] = trunc nuw i8 [[TEMP]] to i1
+    // CHECK: ret i1 [[TRUNC]]
+    intrinsics::unaligned_volatile_load(a)
+}
+
+// CHECK-LABEL: @unaligned_volatile_load_zst
+#[no_mangle]
+pub unsafe fn unaligned_volatile_load_zst(a: *const CustomZst) -> CustomZst {
+    // CHECK: start:
+    // CHECK-NEXT: ret void
+    intrinsics::unaligned_volatile_load(a)
+}
+
+// CHECK-LABEL: @unaligned_volatile_load_array
+// CHECK-SAME: ptr{{.+}}sret([16 x i8]){{.+}}%_0,
+#[no_mangle]
+pub unsafe fn unaligned_volatile_load_array(a: *const [u16; 8]) -> [u16; 8] {
+    // CHECK-NOT: alloca
+    // CHECK: [[TEMP:%.+]] = load volatile i128, ptr %a,
+    // CHECK-SAME: align 1{{,|$}}
+    // CHECK: store i128 [[TEMP]], ptr %_0,
+    // CHECK-SAME: align 2{{,|$}}
+    // CHECK-NEXT: ret void
+    intrinsics::unaligned_volatile_load(a)
+}
+
+// CHECK-LABEL: @unaligned_volatile_load_fat
+#[no_mangle]
+pub unsafe fn unaligned_volatile_load_fat(a: *const UninitFatPointer) -> UninitFatPointer {
+    // CHECK: [[ALLOCA:%.+]] = alloca
+    // CHECK-SAME: [[SIZE]] x i8
+    // CHECK-SAME: align [[ALIGN]]
+
+    // CHECK: [[TEMP:%.+]] = load volatile [[INT]], ptr %a,
+    // CHECK-SAME: align 1{{,|$}}
+    // CHECK: store [[INT]] [[TEMP]], ptr [[ALLOCA]],
+    // CHECK-SAME: align [[ALIGN]]{{,|$}}
+
+    // CHECK: [[T0:%.+]] = load ptr, ptr [[ALLOCA]], align [[ALIGN]]
+    // CHECK: [[T1:%.+]] = getelementptr inbounds i8, ptr [[ALLOCA]]
+    // CHECK: [[T2:%.+]] = load ptr, ptr [[T1]], align [[ALIGN]]
+    // CHECK: [[P1:%.+]] = insertvalue { ptr, ptr } poison, ptr [[T0]], 0
+    // CHECK: [[P2:%.+]] = insertvalue { ptr, ptr } [[P1]], ptr [[T2]], 1
+    // CHECK: ret { ptr, ptr } [[P2]]
     intrinsics::unaligned_volatile_load(a)
 }