Address review comments

rust-lang · Jan 2, 2024 · 8e64fc9 · 8e64fc9
1 parent c2fd26a
commit 8e64fc9
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 9 deletions.
diff --git a/compiler/rustc_codegen_llvm/src/type_of.rs b/compiler/rustc_codegen_llvm/src/type_of.rs
@@ -26,7 +26,6 @@ fn uncached_llvm_type<'a, 'tcx>(
             let element = layout.scalar_llvm_type_at(cx, element);
             return cx.type_vector(element, count);
         }
-        // Treat ScalarPair like a normal aggregate for the purposes of in-memory representation.
         Abi::Uninhabited | Abi::Aggregate { .. } | Abi::ScalarPair(..) => {}
     }
 

diff --git a/compiler/rustc_codegen_ssa/src/mir/place.rs b/compiler/rustc_codegen_ssa/src/mir/place.rs
@@ -108,19 +108,17 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
                     // Also handles the first field of Scalar, ScalarPair, and Vector layouts.
                     self.llval
                 }
-                Abi::ScalarPair(a, b)
-                    if offset == a.size(bx.cx()).align_to(b.align(bx.cx()).abi) =>
-                {
-                    // Offset matches second field.
+                Abi::ScalarPair(..) => {
+                    // FIXME(nikic): Generate this for all ABIs.
                     bx.inbounds_gep(bx.type_i8(), self.llval, &[bx.const_usize(offset.bytes())])
                 }
-                Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } if field.is_zst() => {
+                Abi::Scalar(_) | Abi::Vector { .. } if field.is_zst() => {
                     // ZST fields (even some that require alignment) are not included in Scalar,
                     // ScalarPair, and Vector layouts, so manually offset the pointer.
                     bx.gep(bx.cx().type_i8(), self.llval, &[bx.const_usize(offset.bytes())])
                 }
-                Abi::Scalar(_) | Abi::ScalarPair(..) => {
-                    // All fields of Scalar and ScalarPair layouts must have been handled by this point.
+                Abi::Scalar(_) => {
+                    // All fields of Scalar layouts must have been handled by this point.
                     // Vector layouts have additional fields for each element of the vector, so don't panic in that case.
                     bug!(
                         "offset of non-ZST field `{:?}` does not match layout `{:#?}`",

diff --git a/tests/codegen/zst-offset.rs b/tests/codegen/zst-offset.rs
@@ -22,7 +22,7 @@ pub fn scalar_layout(s: &(u64, ())) {
 // CHECK-LABEL: @scalarpair_layout
 #[no_mangle]
 pub fn scalarpair_layout(s: &(u64, u32, ())) {
-// CHECK: getelementptr i8, {{.+}}, [[USIZE]] 12
+// CHECK: getelementptr inbounds i8, {{.+}}, [[USIZE]] 12
     let x = &s.2;
     witness(&x); // keep variable in an alloca
 }