-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Simplify PartialOrd
on tuples containing primitives
#138135
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 2 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,70 @@ | ||
// MIR for `demo_ge_partial` after PreCodegen | ||
|
||
fn demo_ge_partial(_1: &(f32, f32), _2: &(f32, f32)) -> bool { | ||
debug a => _1; | ||
debug b => _2; | ||
let mut _0: bool; | ||
scope 1 (inlined std::cmp::impls::<impl PartialOrd for &(f32, f32)>::ge) { | ||
scope 2 (inlined core::tuple::<impl PartialOrd for (f32, f32)>::ge) { | ||
let mut _7: std::ops::ControlFlow<bool>; | ||
let _8: bool; | ||
scope 3 { | ||
} | ||
scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<f32> for f32>::spec_chain_ge) { | ||
let mut _3: f32; | ||
let mut _4: f32; | ||
let mut _5: bool; | ||
let mut _6: bool; | ||
scope 5 { | ||
} | ||
} | ||
scope 6 (inlined std::cmp::impls::<impl PartialOrd for f32>::ge) { | ||
let mut _9: f32; | ||
let mut _10: f32; | ||
} | ||
} | ||
} | ||
|
||
bb0: { | ||
StorageLive(_7); | ||
StorageLive(_3); | ||
StorageLive(_4); | ||
_3 = copy ((*_1).0: f32); | ||
_4 = copy ((*_2).0: f32); | ||
StorageLive(_5); | ||
_5 = Eq(copy _3, copy _4); | ||
switchInt(move _5) -> [0: bb1, otherwise: bb2]; | ||
} | ||
|
||
bb1: { | ||
StorageLive(_6); | ||
_6 = Ge(copy _3, copy _4); | ||
_7 = ControlFlow::<bool>::Break(move _6); | ||
StorageDead(_6); | ||
StorageDead(_5); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
_8 = copy ((_7 as Break).0: bool); | ||
_0 = copy _8; | ||
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'm a bit surprised we're not able to make this block _0 = Ge(...) like bb2 ends up as... I'm sure LLVM will work it out though. 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 guess this is #138544 :) 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, that's right. And yes, LLVM will fix it. I'm also working on other changes (#138759 and the unfinished #138582) that'll mean it'll even be fixed in debug codegen, rather than SRoA needing to fix it in LLVM. |
||
goto -> bb3; | ||
} | ||
|
||
bb2: { | ||
StorageDead(_5); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
StorageLive(_9); | ||
_9 = copy ((*_1).1: f32); | ||
StorageLive(_10); | ||
_10 = copy ((*_2).1: f32); | ||
_0 = Ge(move _9, move _10); | ||
StorageDead(_10); | ||
StorageDead(_9); | ||
goto -> bb3; | ||
} | ||
|
||
bb3: { | ||
StorageDead(_7); | ||
return; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// MIR for `demo_le_total` after PreCodegen | ||
|
||
fn demo_le_total(_1: &(u16, i16), _2: &(u16, i16)) -> bool { | ||
debug a => _1; | ||
debug b => _2; | ||
let mut _0: bool; | ||
scope 1 (inlined std::cmp::impls::<impl PartialOrd for &(u16, i16)>::le) { | ||
scope 2 (inlined core::tuple::<impl PartialOrd for (u16, i16)>::le) { | ||
let mut _7: std::ops::ControlFlow<bool>; | ||
let _8: bool; | ||
scope 3 { | ||
} | ||
scope 4 (inlined std::cmp::impls::<impl std::cmp::SpecChainingPartialOrd<u16> for u16>::spec_chain_le) { | ||
let mut _3: u16; | ||
let mut _4: u16; | ||
let mut _5: bool; | ||
let mut _6: bool; | ||
scope 5 { | ||
} | ||
} | ||
scope 6 (inlined std::cmp::impls::<impl PartialOrd for i16>::le) { | ||
let mut _9: i16; | ||
let mut _10: i16; | ||
} | ||
} | ||
} | ||
|
||
bb0: { | ||
StorageLive(_7); | ||
StorageLive(_3); | ||
StorageLive(_4); | ||
_3 = copy ((*_1).0: u16); | ||
_4 = copy ((*_2).0: u16); | ||
StorageLive(_5); | ||
_5 = Eq(copy _3, copy _4); | ||
switchInt(move _5) -> [0: bb1, otherwise: bb2]; | ||
} | ||
|
||
bb1: { | ||
StorageLive(_6); | ||
_6 = Le(copy _3, copy _4); | ||
_7 = ControlFlow::<bool>::Break(move _6); | ||
StorageDead(_6); | ||
StorageDead(_5); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
_8 = copy ((_7 as Break).0: bool); | ||
_0 = copy _8; | ||
goto -> bb3; | ||
} | ||
|
||
bb2: { | ||
StorageDead(_5); | ||
StorageDead(_4); | ||
StorageDead(_3); | ||
StorageLive(_9); | ||
_9 = copy ((*_1).1: i16); | ||
StorageLive(_10); | ||
_10 = copy ((*_2).1: i16); | ||
_0 = Le(move _9, move _10); | ||
StorageDead(_10); | ||
StorageDead(_9); | ||
goto -> bb3; | ||
} | ||
|
||
bb3: { | ||
StorageDead(_7); | ||
return; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
//@ compile-flags: -O -Zmir-opt-level=2 -Cdebuginfo=0 | ||
//@ needs-unwind | ||
|
||
#![crate_type = "lib"] | ||
|
||
// EMIT_MIR tuple_ord.demo_le_total.PreCodegen.after.mir | ||
pub fn demo_le_total(a: &(u16, i16), b: &(u16, i16)) -> bool { | ||
// CHECK-LABEL: demo_le_total | ||
a <= b | ||
} | ||
|
||
// EMIT_MIR tuple_ord.demo_ge_partial.PreCodegen.after.mir | ||
pub fn demo_ge_partial(a: &(f32, f32), b: &(f32, f32)) -> bool { | ||
// CHECK-LABEL: demo_ge_partial | ||
a >= b | ||
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. Doh, I somehow managed to not notice that I'd put Sorry for the slightly-worse diff; it doesn't really change anything material though. |
||
} |
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.
Is it worse for compile times or similar to make these (unstable) provided methods? If we can avoid another use of specialization that seems worthwhile to me - I forget if core's usage is guaranteed sound or not (I seem to recall some gaps)...
Uh oh!
There was an error while loading. Please reload this page.
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.
I think this usage is sound, since we're only specializing on primitives that don't have lifetimes. But I was torn between the two anyway, so if you have a weak preference for the other way I'm happy to give that a shot. Let's see how it comes out. I always like less specialization 🙂
@rustbot author