-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Improve the generic MIR in the default PartialOrd::le
and friends
#137904
Changes from all commits
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,89 @@ | ||
// MIR for `demo_le` after PreCodegen | ||
|
||
fn demo_le(_1: &MultiField, _2: &MultiField) -> bool { | ||
debug a => _1; | ||
debug b => _2; | ||
let mut _0: bool; | ||
scope 1 (inlined <MultiField as PartialOrd>::le) { | ||
let mut _11: std::option::Option<std::cmp::Ordering>; | ||
scope 2 (inlined Option::<std::cmp::Ordering>::is_some_and::<fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le}>) { | ||
let _12: std::cmp::Ordering; | ||
scope 3 { | ||
scope 4 (inlined <fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le} as FnOnce<(std::cmp::Ordering,)>>::call_once - shim(fn(std::cmp::Ordering) -> bool {std::cmp::Ordering::is_le})) { | ||
scope 5 (inlined std::cmp::Ordering::is_le) { | ||
let mut _13: i8; | ||
scope 6 (inlined std::cmp::Ordering::as_raw) { | ||
} | ||
} | ||
} | ||
} | ||
} | ||
scope 7 (inlined <MultiField as PartialOrd>::partial_cmp) { | ||
let mut _6: std::option::Option<std::cmp::Ordering>; | ||
let mut _7: i8; | ||
scope 8 { | ||
} | ||
scope 9 (inlined std::cmp::impls::<impl PartialOrd for char>::partial_cmp) { | ||
let mut _3: char; | ||
let mut _4: char; | ||
let mut _5: std::cmp::Ordering; | ||
} | ||
scope 10 (inlined std::cmp::impls::<impl PartialOrd for i16>::partial_cmp) { | ||
let mut _8: i16; | ||
let mut _9: i16; | ||
let mut _10: std::cmp::Ordering; | ||
} | ||
} | ||
} | ||
|
||
bb0: { | ||
StorageLive(_12); | ||
StorageLive(_11); | ||
StorageLive(_5); | ||
StorageLive(_7); | ||
StorageLive(_3); | ||
_3 = copy ((*_1).0: char); | ||
StorageLive(_4); | ||
_4 = copy ((*_2).0: char); | ||
_5 = Cmp(move _3, move _4); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
_6 = Option::<std::cmp::Ordering>::Some(copy _5); | ||
_7 = discriminant(_5); | ||
switchInt(move _7) -> [0: bb1, otherwise: bb2]; | ||
} | ||
|
||
bb1: { | ||
StorageLive(_10); | ||
StorageLive(_8); | ||
_8 = copy ((*_1).1: i16); | ||
StorageLive(_9); | ||
_9 = copy ((*_2).1: i16); | ||
_10 = Cmp(move _8, move _9); | ||
StorageDead(_9); | ||
StorageDead(_8); | ||
_11 = Option::<std::cmp::Ordering>::Some(move _10); | ||
StorageDead(_10); | ||
StorageDead(_7); | ||
StorageDead(_5); | ||
goto -> bb3; | ||
} | ||
|
||
bb2: { | ||
_11 = copy _6; | ||
StorageDead(_7); | ||
StorageDead(_5); | ||
goto -> bb3; | ||
} | ||
|
||
bb3: { | ||
_12 = move ((_11 as Some).0: std::cmp::Ordering); | ||
StorageLive(_13); | ||
_13 = discriminant(_12); | ||
_0 = Le(move _13, const 0_i8); | ||
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. Using this implementation for The previous _14 = Eq(copy _13, const 1_i8);
_0 = Not(move _14); which is much less obviously a (That should also make these look like what you'd get in C++ for things like 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. Huh. Do we have a MIR transform that would normalize 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. No, because we don't have an InstCombine system. GVN can't do it for two reasons:
EDIT: err, I replied about making a 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. Ah, I see. |
||
StorageDead(_13); | ||
StorageDead(_11); | ||
StorageDead(_12); | ||
return; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,33 @@ | ||
// skip-filecheck | ||
//@ compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=0 | ||
|
||
#![crate_type = "lib"] | ||
|
||
#[derive(PartialOrd, PartialEq)] | ||
pub struct MultiField(char, i16); | ||
|
||
// Because this isn't derived by the impl, it's not on the `{impl#0}-partial_cmp`, | ||
// and thus we need to call it to see what the inlined generic one produces. | ||
pub fn demo_le(a: &MultiField, b: &MultiField) -> bool { | ||
// CHECK-LABEL: fn demo_le | ||
// CHECK: inlined <MultiField as PartialOrd>::le | ||
// CHECK: inlined{{.+}}is_some_and | ||
// CHECK: inlined <MultiField as PartialOrd>::partial_cmp | ||
|
||
// CHECK: [[A0:_[0-9]+]] = copy ((*_1).0: char); | ||
// CHECK: [[B0:_[0-9]+]] = copy ((*_2).0: char); | ||
// CHECK: Cmp(move [[A0]], move [[B0]]); | ||
|
||
// CHECK: [[D0:_[0-9]+]] = discriminant({{.+}}); | ||
// CHECK: switchInt(move [[D0]]) -> [0: bb{{[0-9]+}}, otherwise: bb{{[0-9]+}}]; | ||
|
||
// CHECK: [[A1:_[0-9]+]] = copy ((*_1).1: i16); | ||
// CHECK: [[B1:_[0-9]+]] = copy ((*_2).1: i16); | ||
// CHECK: Cmp(move [[A1]], move [[B1]]); | ||
|
||
// CHECK: [[D1:_[0-9]+]] = discriminant({{.+}}); | ||
// CHECK: _0 = Le(move [[D1]], const 0_i8); | ||
*a <= *b | ||
} | ||
|
||
// EMIT_MIR derived_ord.{impl#0}-partial_cmp.PreCodegen.after.mir | ||
// EMIT_MIR derived_ord.demo_le.PreCodegen.after.mir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious... how does this differ from an
as
cast? (ie.self as i8
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
as
here would stick twoassume
s in the MIR for no good reason.I need to go remove those now that LLVM has
range
attributes, don't I...