Skip to content

Commit 620bc57

Browse files
borsbors
committed
Auto merge of #121668 - erikdesjardins:commonprim, r=<try>
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 #97540. r? `@ghost`
2 parents 71ffdf7 + 4dabbcb commit 620bc57

File tree

7 files changed

+221
-19
lines changed

7 files changed

+221
-19
lines changed

‎compiler/rustc_abi/src/layout.rs‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -813,15 +813,44 @@ where
813813
break;
814814
}
815815
};
816-
if let Some(pair) = common_prim {
817-
// This is pretty conservative. We could go fancier
818-
// by conflating things like i32 and u32, or even
819-
// realising that (u8, u8) could just cohabit with
820-
// u16 or even u32.
821-
if pair != (prim, offset) {
816+
if let Some((old_common_prim, common_offset)) = common_prim {
817+
// All variants must be at the same offset
818+
if offset != common_offset {
822819
common_prim = None;
823820
break;
824821
}
822+
// This is pretty conservative. We could go fancier
823+
// by realising that (u8, u8) could just cohabit with
824+
// u16 or even u32.
825+
let new_common_prim = match (old_common_prim, prim) {
826+
// Allow all identical primitives.
827+
(x, y) if x == y => Some(x),
828+
// Allow integers of the same size with differing signedness.
829+
// We arbitrarily choose the signedness of the first variant.
830+
(p @ Primitive::Int(x, _), Primitive::Int(y, _)) if x == y => Some(p),
831+
// Allow integers mixed with pointers of the same layout.
832+
// We must represent this using a pointer, to avoid
833+
// roundtripping pointers through ptrtoint/inttoptr.
834+
(p @ Primitive::Pointer(_), i @ Primitive::Int(..))
835+
| (i @ Primitive::Int(..), p @ Primitive::Pointer(_))
836+
if p.size(dl) == i.size(dl) && p.align(dl) == i.align(dl) =>
837+
{
838+
Some(p)
839+
}
840+
_ => None,
841+
};
842+
match new_common_prim {
843+
Some(new_prim) => {
844+
// Primitives are compatible.
845+
// We may be updating the primitive here, for example from int->ptr.
846+
common_prim = Some((new_prim, common_offset));
847+
}
848+
None => {
849+
// Primitives are incompatible.
850+
common_prim = None;
851+
break;
852+
}
853+
}
825854
} else {
826855
common_prim = Some((prim, offset));
827856
}

‎tests/codegen/common_prim_int_ptr.rs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//@ compile-flags: -O
2+
3+
#![crate_type = "lib"]
4+
5+
// Tests that codegen works properly when enums like `Result<usize, Box<()>>`
6+
// are represented as `{ u64, ptr }`, i.e., for `Ok(123)`, `123` is stored
7+
// as a pointer.
8+
9+
// CHECK-LABEL: @insert_int
10+
#[no_mangle]
11+
pub fn insert_int(x: usize) -> Result<usize, Box<()>> {
12+
// CHECK: start:
13+
// CHECK-NEXT: inttoptr i{{[0-9]+}} %x to ptr
14+
// CHECK-NEXT: insertvalue
15+
// CHECK-NEXT: ret { i{{[0-9]+}}, ptr }
16+
Ok(x)
17+
}
18+
19+
// CHECK-LABEL: @insert_box
20+
#[no_mangle]
21+
pub fn insert_box(x: Box<()>) -> Result<usize, Box<()>> {
22+
// CHECK: start:
23+
// CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
24+
// CHECK-NEXT: ret
25+
Err(x)
26+
}
27+
28+
// CHECK-LABEL: @extract_int
29+
#[no_mangle]
30+
pub unsafe fn extract_int(x: Result<usize, Box<()>>) -> usize {
31+
// CHECK: start:
32+
// CHECK-NEXT: ptrtoint
33+
// CHECK-NEXT: ret
34+
x.unwrap_unchecked()
35+
}
36+
37+
// CHECK-LABEL: @extract_box
38+
#[no_mangle]
39+
pub unsafe fn extract_box(x: Result<usize, Box<()>>) -> Box<()> {
40+
// CHECK: start:
41+
// CHECK-NEXT: ret ptr
42+
x.unwrap_err_unchecked()
43+
}

‎tests/codegen/try_question_mark_nop.rs‎

Lines changed: 92 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,41 @@
44
#![crate_type = "lib"]
55
#![feature(try_blocks)]
66

7-
// These are now NOPs in LLVM 15, presumably thanks to nikic's change mentioned in
8-
// <https://github.com/rust-lang/rust/issues/85133#issuecomment-1072168354>.
9-
// Unfortunately, as of 2022-08-17 they're not yet nops for `u64`s nor `Option`.
10-
117
use std::ops::ControlFlow::{self, Continue, Break};
8+
use std::ptr::NonNull;
9+
10+
// CHECK-LABEL: @option_nop_match_32
11+
#[no_mangle]
12+
pub fn option_nop_match_32(x: Option<u32>) -> Option<u32> {
13+
// CHECK: start:
14+
// CHECK-NEXT: insertvalue { i32, i32 }
15+
// CHECK-NEXT: insertvalue { i32, i32 }
16+
// CHECK-NEXT: ret { i32, i32 }
17+
match x {
18+
Some(x) => Some(x),
19+
None => None,
20+
}
21+
}
22+
23+
// CHECK-LABEL: @option_nop_traits_32
24+
#[no_mangle]
25+
pub fn option_nop_traits_32(x: Option<u32>) -> Option<u32> {
26+
// CHECK: start:
27+
// CHECK-NEXT: insertvalue { i32, i32 }
28+
// CHECK-NEXT: insertvalue { i32, i32 }
29+
// CHECK-NEXT: ret { i32, i32 }
30+
try {
31+
x?
32+
}
33+
}
1234

1335
// CHECK-LABEL: @result_nop_match_32
1436
#[no_mangle]
1537
pub fn result_nop_match_32(x: Result<i32, u32>) -> Result<i32, u32> {
16-
// CHECK: start
17-
// CHECK-NEXT: ret i64 %0
38+
// CHECK: start:
39+
// CHECK-NEXT: insertvalue { i32, i32 }
40+
// CHECK-NEXT: insertvalue { i32, i32 }
41+
// CHECK-NEXT: ret { i32, i32 }
1842
match x {
1943
Ok(x) => Ok(x),
2044
Err(x) => Err(x),
@@ -24,8 +48,60 @@ pub fn result_nop_match_32(x: Result<i32, u32>) -> Result<i32, u32> {
2448
// CHECK-LABEL: @result_nop_traits_32
2549
#[no_mangle]
2650
pub fn result_nop_traits_32(x: Result<i32, u32>) -> Result<i32, u32> {
27-
// CHECK: start
28-
// CHECK-NEXT: ret i64 %0
51+
// CHECK: start:
52+
// CHECK-NEXT: insertvalue { i32, i32 }
53+
// CHECK-NEXT: insertvalue { i32, i32 }
54+
// CHECK-NEXT: ret { i32, i32 }
55+
try {
56+
x?
57+
}
58+
}
59+
60+
// CHECK-LABEL: @result_nop_match_64
61+
#[no_mangle]
62+
pub fn result_nop_match_64(x: Result<i64, u64>) -> Result<i64, u64> {
63+
// CHECK: start:
64+
// CHECK-NEXT: insertvalue { i64, i64 }
65+
// CHECK-NEXT: insertvalue { i64, i64 }
66+
// CHECK-NEXT: ret { i64, i64 }
67+
match x {
68+
Ok(x) => Ok(x),
69+
Err(x) => Err(x),
70+
}
71+
}
72+
73+
// CHECK-LABEL: @result_nop_traits_64
74+
#[no_mangle]
75+
pub fn result_nop_traits_64(x: Result<i64, u64>) -> Result<i64, u64> {
76+
// CHECK: start:
77+
// CHECK-NEXT: insertvalue { i64, i64 }
78+
// CHECK-NEXT: insertvalue { i64, i64 }
79+
// CHECK-NEXT: ret { i64, i64 }
80+
try {
81+
x?
82+
}
83+
}
84+
85+
// CHECK-LABEL: @result_nop_match_ptr
86+
#[no_mangle]
87+
pub fn result_nop_match_ptr(x: Result<usize, Box<()>>) -> Result<usize, Box<()>> {
88+
// CHECK: start:
89+
// CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
90+
// CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
91+
// CHECK-NEXT: ret
92+
match x {
93+
Ok(x) => Ok(x),
94+
Err(x) => Err(x),
95+
}
96+
}
97+
98+
// CHECK-LABEL: @result_nop_traits_ptr
99+
#[no_mangle]
100+
pub fn result_nop_traits_ptr(x: Result<u64, NonNull<()>>) -> Result<u64, NonNull<()>> {
101+
// CHECK: start:
102+
// CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
103+
// CHECK-NEXT: insertvalue { i{{[0-9]+}}, ptr }
104+
// CHECK-NEXT: ret
29105
try {
30106
x?
31107
}
@@ -34,8 +110,10 @@ pub fn result_nop_traits_32(x: Result<i32, u32>) -> Result<i32, u32> {
34110
// CHECK-LABEL: @control_flow_nop_match_32
35111
#[no_mangle]
36112
pub fn control_flow_nop_match_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u32> {
37-
// CHECK: start
38-
// CHECK-NEXT: ret i64 %0
113+
// CHECK: start:
114+
// CHECK-NEXT: insertvalue { i32, i32 }
115+
// CHECK-NEXT: insertvalue { i32, i32 }
116+
// CHECK-NEXT: ret { i32, i32 }
39117
match x {
40118
Continue(x) => Continue(x),
41119
Break(x) => Break(x),
@@ -45,8 +123,10 @@ pub fn control_flow_nop_match_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u
45123
// CHECK-LABEL: @control_flow_nop_traits_32
46124
#[no_mangle]
47125
pub fn control_flow_nop_traits_32(x: ControlFlow<i32, u32>) -> ControlFlow<i32, u32> {
48-
// CHECK: start
49-
// CHECK-NEXT: ret i64 %0
126+
// CHECK: start:
127+
// CHECK-NEXT: insertvalue { i32, i32 }
128+
// CHECK-NEXT: insertvalue { i32, i32 }
129+
// CHECK-NEXT: ret { i32, i32 }
50130
try {
51131
x?
52132
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//@ normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
2+
//@ normalize-stderr-test "Int\(I[0-9]+," -> "Int(I?,"
3+
//@ normalize-stderr-test "valid_range: 0..=[0-9]+" -> "valid_range: $$VALID_RANGE"
4+
5+
//! Enum layout tests related to scalar pairs with an int/ptr common primitive.
6+
7+
#![feature(rustc_attrs)]
8+
#![feature(never_type)]
9+
#![crate_type = "lib"]
10+
11+
#[rustc_layout(abi)]
12+
enum ScalarPairPointerWithInt { //~ERROR: abi: ScalarPair
13+
A(usize),
14+
B(Box<()>),
15+
}
16+
17+
// Negative test--ensure that pointers are not commoned with integers
18+
// of a different size. (Assumes that no target has 8 bit pointers, which
19+
// feels pretty safe.)
20+
#[rustc_layout(abi)]
21+
enum NotScalarPairPointerWithSmallerInt { //~ERROR: abi: Aggregate
22+
A(u8),
23+
B(Box<()>),
24+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: abi: ScalarPair(Initialized { value: Int(I?, false), valid_range: $VALID_RANGE }, Initialized { value: Pointer(AddressSpace(0)), valid_range: $VALID_RANGE })
2+
--> $DIR/enum-scalar-pair-int-ptr.rs:12:1
3+
|
4+
LL | enum ScalarPairPointerWithInt {
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: abi: Aggregate { sized: true }
8+
--> $DIR/enum-scalar-pair-int-ptr.rs:21:1
9+
|
10+
LL | enum NotScalarPairPointerWithSmallerInt {
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 2 previous errors
14+

‎tests/ui/layout/enum.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,9 @@ enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)
1616
A,
1717
B([u8; 15], !), // make sure there is space being reserved for this field.
1818
}
19+
20+
#[rustc_layout(abi)]
21+
enum ScalarPairDifferingSign { //~ERROR: abi: ScalarPair
22+
A(u64),
23+
B(i64),
24+
}

‎tests/ui/layout/enum.stderr‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,11 @@ error: size: Size(16 bytes)
1010
LL | enum UninhabitedVariantSpace {
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1212

13-
error: aborting due to 2 previous errors
13+
error: abi: ScalarPair(Initialized { value: Int(I64, false), valid_range: 0..=1 }, Initialized { value: Int(I64, false), valid_range: 0..=18446744073709551615 })
14+
--> $DIR/enum.rs:21:1
15+
|
16+
LL | enum ScalarPairDifferingSign {
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
19+
error: aborting due to 3 previous errors
1420

0 commit comments

Comments
 (0)