Repository navigation
derived PartialOrd::le is non-optimal for 2-field struct of primitive integers #106107
Description
This codegen test should pass: comparison-operators-twofields.patch https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=4c8846efffd0574ec44e60c3cffbe842
And it turns out that the derived PartialOrd::lt does pass that test
define noundef zeroext i1 @check_lt(i16 %0, i16 %1, i16 %2, i16 %3) unnamed_addr #0 {
start:
%_6.i.i = icmp slt i16 %0, %2
%_9.i.i.not = icmp eq i16 %0, %2
%_6.i3.i = icmp ult i16 %1, %3
%4 = select i1 %_9.i.i.not, i1 %_6.i3.i, i1 %_6.i.i
ret i1 %4
}but the derived le, gt, and ge do not pass it.
define noundef zeroext i1 @check_gt(i16 %0, i16 %1, i16 %2, i16 %3) unnamed_addr #0 {
start:
%_6.i.i = icmp slt i16 %0, %2
%_9.i.i = icmp ne i16 %0, %2
%..i.i = zext i1 %_9.i.i to i8
%_3.0.i.i = select i1 %_6.i.i, i8 -1, i8 %..i.i
%4 = icmp eq i8 %_3.0.i.i, 0
%_6.i3.i = icmp ult i16 %1, %3
%_9.i4.i = icmp ne i16 %1, %3
%..i5.i = zext i1 %_9.i4.i to i8
%_3.0.i6.i = select i1 %_6.i3.i, i8 -1, i8 %..i5.i
%.0.i = select i1 %4, i8 %_3.0.i6.i, i8 %_3.0.i.i
%5 = icmp eq i8 %.0.i, 1
ret i1 %5
}instead leaving in a whole bunch of unnecessary stuff.
Given the direction in #98655, we probably don't want to fix this by changing the derive.
So potential fixes could be changing our Ord::cmp implementation for integers (cc #105840), changing the default PartialOrd::le implementations (cc #106065), improved optimizations in LLVM (cc llvm/llvm-project#59666), or more.
Activity
scottmcm commentedon Feb 17, 2023
I tried switching the
cmpimplementation for integers to use the same order as clang (see llvm/llvm-project#60012), which managed to fixle, but didn't fixgtorge, so it's not obvious that it's worth doing as it's more x86 asm for a singlecmp.If you want to play with this yourself, there's a codegen test in https://github.com/rust-lang/rust/pull/108156/files#diff-9ee4982898ebe1fa1f721acc54dc662f4a51cb2e017c71f9e24286493358e020
But we might just need to wait for LLVM 17 and llvm/llvm-project#59666 (comment).
Optimise `Entity` with repr align & manual `PartialOrd`/`Ord` (#10558)
Optimise `Entity` with repr align & manual `PartialOrd`/`Ord` (bevyen…
Rollup merge of rust-lang#137197 - scottmcm:cmp-20, r=ibraheemdev
Rollup merge of rust-lang#137197 - scottmcm:cmp-20, r=ibraheemdev
Rollup merge of rust-lang#137197 - scottmcm:cmp-20, r=ibraheemdev
Unrolled build for rust-lang#137197
Rollup merge of rust-lang#137197 - scottmcm:cmp-20, r=ibraheemdev
Rollup merge of rust-lang#137197 - scottmcm:cmp-20, r=ibraheemdev