Skip to content

Commit

Permalink
Auto merge of rust-lang#121668 - erikdesjardins:commonprim, r=scottmc…
Browse files Browse the repository at this point in the history
…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`
  • Loading branch information
bors committed Mar 13, 2024
2 parents 184c5ab + 9f55200 commit 3cbb932
Show file tree
Hide file tree
Showing 7 changed files with 222 additions and 19 deletions.
34 changes: 28 additions & 6 deletions compiler/rustc_abi/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
51 changes: 51 additions & 0 deletions tests/codegen/common_prim_int_ptr.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
104 changes: 92 additions & 12 deletions tests/codegen/try_question_mark_nop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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?
}
Expand All @@ -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),
Expand All @@ -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?
}
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/layout/enum-scalar-pair-int-ptr.rs
Original file line number Diff line number Diff line change
@@ -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<()>),
}
14 changes: 14 additions & 0 deletions tests/ui/layout/enum-scalar-pair-int-ptr.stderr
Original file line number Diff line number Diff line change
@@ -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

6 changes: 6 additions & 0 deletions tests/ui/layout/enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
8 changes: 7 additions & 1 deletion tests/ui/layout/enum.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 3cbb932

Please sign in to comment.