Auto merge of rust-lang#121668 - erikdesjardins:commonprim, r=scottmc…

…m,oli-obk

Represent `Result<usize, Box<T>>` as ScalarPair(i64, ptr)

This allows types like `Result<usize, std::io::Error>` (and integers of differing sign, e.g. `Result<u64, i64>`) to be passed in a pair of registers instead of through memory, like `Result<u64, u64>` or `Result<Box<T>, Box<U>>` are today.

Fixes rust-lang#97540.

r? `@ghost`

compiler/rustc_abi/src/layout.rs

@@ -816,15 +816,37 @@ where
                     break;
                 }
             };
-            if let Some(pair) = common_prim {
-                // This is pretty conservative. We could go fancier
-                // by conflating things like i32 and u32, or even
-                // realising that (u8, u8) could just cohabit with
-                // u16 or even u32.
-                if pair != (prim, offset) {
+            if let Some((old_prim, common_offset)) = common_prim {
+                // All variants must be at the same offset
+                if offset != common_offset {
                     common_prim = None;
                     break;
                 }
+                // This is pretty conservative. We could go fancier
+                // by realising that (u8, u8) could just cohabit with
+                // u16 or even u32.
+                let new_prim = match (old_prim, prim) {
+                    // Allow all identical primitives.
+                    (x, y) if x == y => x,
+                    // Allow integers of the same size with differing signedness.
+                    // We arbitrarily choose the signedness of the first variant.
+                    (p @ Primitive::Int(x, _), Primitive::Int(y, _)) if x == y => p,
+                    // Allow integers mixed with pointers of the same layout.
+                    // We must represent this using a pointer, to avoid
+                    // roundtripping pointers through ptrtoint/inttoptr.
+                    (p @ Primitive::Pointer(_), i @ Primitive::Int(..))
+                    | (i @ Primitive::Int(..), p @ Primitive::Pointer(_))
+                        if p.size(dl) == i.size(dl) && p.align(dl) == i.align(dl) =>
+                    {
+                        p
+                    }
+                    _ => {
+                        common_prim = None;
+                        break;
+                    }
+                };
+                // We may be updating the primitive here, for example from int->ptr.
+                common_prim = Some((new_prim, common_offset));
             } else {
                 common_prim = Some((prim, offset));
             }

tests/codegen/common_prim_int_ptr.rs

@@ -0,0 +1,51 @@
+//@ compile-flags: -O
+
+#![crate_type = "lib"]
+#![feature(core_intrinsics)]
+
+// Tests that codegen works properly when enums like `Result<usize, Box<()>>`
+// are represented as `{ u64, ptr }`, i.e., for `Ok(123)`, `123` is stored
+// as a pointer.
+
+// CHECK-LABEL: @insert_int
+#[no_mangle]
+pub fn insert_int(x: usize) -> Result<usize, Box<()>> {
+    // CHECK: start:
+    // CHECK-NEXT: inttoptr i{{[0-9]+}} %x to ptr
+    // CHECK-NEXT: insertvalue
+    // CHECK-NEXT: ret { i{{[0-9]+}}, ptr }
+    Ok(x)
+}
+
+// CHECK-LABEL: @insert_box
+#[no_mangle]
+pub fn insert_box(x: Box<()>) -> Result<usize, Box<()>> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
+    // CHECK-NEXT: ret
+    Err(x)
+}
+
+// CHECK-LABEL: @extract_int
+// CHECK-NOT: nonnull
+// CHECK-SAME: (i{{[0-9]+}} {{[^,]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
+#[no_mangle]
+pub unsafe fn extract_int(x: Result<usize, Box<()>>) -> usize {
+    // CHECK: [[TEMP:%.+]] = ptrtoint ptr [[PAYLOAD]] to [[USIZE:i[0-9]+]]
+    // CHECK: ret [[USIZE]] [[TEMP]]
+    match x {
+        Ok(v) => v,
+        Err(_) => std::intrinsics::unreachable(),
+    }
+}
+
+// CHECK-LABEL: @extract_box
+// CHECK-SAME: (i{{[0-9]+}} {{[^,]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
+#[no_mangle]
+pub unsafe fn extract_box(x: Result<usize, Box<i32>>) -> Box<i32> {
+    // CHECK: ret ptr [[PAYLOAD]]
+    match x {
+        Ok(_) => std::intrinsics::unreachable(),
+        Err(e) => e,
+    }
+}

tests/codegen/try_question_mark_nop.rs

@@ -4,17 +4,41 @@
 #![crate_type = "lib"]
 #![feature(try_blocks)]
 
-// These are now NOPs in LLVM 15, presumably thanks to nikic's change mentioned in
-// <https://github.com/rust-lang/rust/issues/85133#issuecomment-1072168354>.
-// Unfortunately, as of 2022-08-17 they're not yet nops for `u64`s nor `Option`.
-
 use std::ops::ControlFlow::{self, Continue, Break};
+use std::ptr::NonNull;
+
+// CHECK-LABEL: @option_nop_match_32
+#[no_mangle]
+pub fn option_nop_match_32(x: Option<u32>) -> Option<u32> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
+    match x {
+        Some(x) => Some(x),
+        None => None,
+    }
+}
+
+// CHECK-LABEL: @option_nop_traits_32
+#[no_mangle]
+pub fn option_nop_traits_32(x: Option<u32>) -> Option<u32> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
+    try {
+        x?
+    }
+}
 
 // CHECK-LABEL: @result_nop_match_32
 #[no_mangle]
 pub fn result_nop_match_32(x: Result<i32, u32>) -> Result<i32, u32> {
-    // CHECK: start
-    // CHECK-NEXT: ret i64 %0
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
     match x {
         Ok(x) => Ok(x),
         Err(x) => Err(x),
@@ -24,8 +48,60 @@ pub fn result_nop_match_32(x: Result<i32, u32>) -> Result<i32, u32> {
 // CHECK-LABEL: @result_nop_traits_32
 #[no_mangle]
 pub fn result_nop_traits_32(x: Result<i32, u32>) -> Result<i32, u32> {
-    // CHECK: start
-    // CHECK-NEXT: ret i64 %0
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
+    try {
+        x?
+    }
+}
+
+// CHECK-LABEL: @result_nop_match_64
+#[no_mangle]
+pub fn result_nop_match_64(x: Result<i64, u64>) -> Result<i64, u64> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i64, i64 }
+    // CHECK-NEXT: insertvalue { i64, i64 }
+    // CHECK-NEXT: ret { i64, i64 }
+    match x {
+        Ok(x) => Ok(x),
+        Err(x) => Err(x),
+    }
+}
+
+// CHECK-LABEL: @result_nop_traits_64
+#[no_mangle]
+pub fn result_nop_traits_64(x: Result<i64, u64>) -> Result<i64, u64> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i64, i64 }
+    // CHECK-NEXT: insertvalue { i64, i64 }
+    // CHECK-NEXT: ret { i64, i64 }
+    try {
+        x?
+    }
+}
+
+// CHECK-LABEL: @result_nop_match_ptr
+#[no_mangle]
+pub fn result_nop_match_ptr(x: Result<usize, Box<()>>) -> Result<usize, Box<()>> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
+    // CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
+    // CHECK-NEXT: ret
+    match x {
+        Ok(x) => Ok(x),
+        Err(x) => Err(x),
+    }
+}
+
+// CHECK-LABEL: @result_nop_traits_ptr
+#[no_mangle]
+pub fn result_nop_traits_ptr(x: Result<u64, NonNull<()>>) -> Result<u64, NonNull<()>> {
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
+    // CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
+    // CHECK-NEXT: ret
     try {
         x?
     }
@@ -34,8 +110,10 @@ pub fn result_nop_traits_32(x: Result<i32, u32>) -> Result<i32, u32> {
 // CHECK-LABEL: @control_flow_nop_match_32
 #[no_mangle]
 pub fn control_flow_nop_match_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u32> {
-    // CHECK: start
-    // CHECK-NEXT: ret i64 %0
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
     match x {
         Continue(x) => Continue(x),
         Break(x) => Break(x),
@@ -45,8 +123,10 @@ pub fn control_flow_nop_match_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u
 // CHECK-LABEL: @control_flow_nop_traits_32
 #[no_mangle]
 pub fn control_flow_nop_traits_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u32> {
-    // CHECK: start
-    // CHECK-NEXT: ret i64 %0
+    // CHECK: start:
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: insertvalue { i32, i32 }
+    // CHECK-NEXT: ret { i32, i32 }
     try {
         x?
     }

tests/ui/layout/enum-scalar-pair-int-ptr.rs

@@ -0,0 +1,24 @@
+//@ normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
+//@ normalize-stderr-test "Int\(I[0-9]+," -> "Int(I?,"
+//@ normalize-stderr-test "valid_range: 0..=[0-9]+" -> "valid_range: $$VALID_RANGE"
+
+//! Enum layout tests related to scalar pairs with an int/ptr common primitive.
+
+#![feature(rustc_attrs)]
+#![feature(never_type)]
+#![crate_type = "lib"]
+
+#[rustc_layout(abi)]
+enum ScalarPairPointerWithInt { //~ERROR: abi: ScalarPair
+    A(usize),
+    B(Box<()>),
+}
+
+// Negative test--ensure that pointers are not commoned with integers
+// of a different size. (Assumes that no target has 8 bit pointers, which
+// feels pretty safe.)
+#[rustc_layout(abi)]
+enum NotScalarPairPointerWithSmallerInt { //~ERROR: abi: Aggregate
+    A(u8),
+    B(Box<()>),
+}

tests/ui/layout/enum-scalar-pair-int-ptr.stderr

@@ -0,0 +1,14 @@
+error: abi: ScalarPair(Initialized { value: Int(I?, false), valid_range: $VALID_RANGE }, Initialized { value: Pointer(AddressSpace(0)), valid_range: $VALID_RANGE })
+  --> $DIR/enum-scalar-pair-int-ptr.rs:12:1
+   |
+LL | enum ScalarPairPointerWithInt {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: abi: Aggregate { sized: true }
+  --> $DIR/enum-scalar-pair-int-ptr.rs:21:1
+   |
+LL | enum NotScalarPairPointerWithSmallerInt {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/layout/enum.rs

@@ -16,3 +16,9 @@ enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)
     A,
     B([u8; 15], !), // make sure there is space being reserved for this field.
 }
+
+#[rustc_layout(abi)]
+enum ScalarPairDifferingSign { //~ERROR: abi: ScalarPair
+    A(u8),
+    B(i8),
+}

tests/ui/layout/enum.stderr

@@ -10,5 +10,11 @@ error: size: Size(16 bytes)
 LL | enum UninhabitedVariantSpace {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: abi: ScalarPair(Initialized { value: Int(I8, false), valid_range: 0..=1 }, Initialized { value: Int(I8, false), valid_range: 0..=255 })
+  --> $DIR/enum.rs:21:1
+   |
+LL | enum ScalarPairDifferingSign {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors