-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Represent Result<usize, Box<T>>
as ScalarPair(i64, ptr)
#121668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
4dabbcb
f574c65
8e40b17
5ccada6
9f55200
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
//@ compile-flags: -O | ||
|
||
#![crate_type = "lib"] | ||
|
||
// 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) | ||
} | ||
Comment on lines
+10
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am surprised that this just works, inserting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is optimized IR, so it's likely that rustc did something much more boring but LLVM optimized it to this. Add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, you get an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Rust would actually prefer this example to be a GEP off null, like #121242, I think. Not that this PR would need to do that.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How did I forget that... Yeah, the IR we generate just spills to an alloca. |
||
|
||
// 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 | ||
#[no_mangle] | ||
pub unsafe fn extract_int(x: Result<usize, Box<()>>) -> usize { | ||
// CHECK: start: | ||
// CHECK-NEXT: ptrtoint | ||
// CHECK-NEXT: ret | ||
x.unwrap_unchecked() | ||
} | ||
|
||
// CHECK-LABEL: @extract_box | ||
#[no_mangle] | ||
pub unsafe fn extract_box(x: Result<usize, Box<()>>) -> Box<()> { | ||
// CHECK: start: | ||
// CHECK-NEXT: ret ptr | ||
x.unwrap_err_unchecked() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 } | ||
Comment on lines
35
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although this is no longer just a For real, non-noop code, the scalar pair layout is arguably slightly better, since passing the components in separate arguments means that consuming code can just use the second argument directly, instead of having to shift it down from the top 32 bits of an i64 reg. It was kinda janky in the first place for this test to rely on the fact that integers with differing signedness don't get scalar pair layout; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, agreed it's fine to change this test like this. The important part is that it's not branching, not |
||
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? | ||
} | ||
|
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<()>), | ||
} |
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 | ||
|
Uh oh!
There was an error while loading. Please reload this page.