Conversation
zeegomo
force-pushed
the
drop-if
branch
3 times, most recently
from
798fee2 to
afdec3c
Compare
zeegomo
force-pushed
the
drop-elab
branch
3 times, most recently
from
2303e04 to
54adf94
Compare
zeegomo
force-pushed
the
drop-elab
branch
2 times, most recently
from
67da872 to
25cf2b6
Compare
zeegomo
pushed a commit
that referenced
this pull request
Feb 4, 2023
…u-se
Implement `SpecOptionPartialEq` for `cmp::Ordering`
Noticed as I continue to explore options for having code using `partial_cmp` optimize better.
Before:
```llvm
; Function Attrs: mustprogress nofree nosync nounwind willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #0 {
start:
%2 = icmp eq i8 %0, 2
br i1 %2, label %bb1.i, label %bb3.i
bb1.i: ; preds = %start
%3 = icmp eq i8 %1, 2
br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"
bb3.i: ; preds = %start
%.not.i = icmp ne i8 %1, 2
%4 = icmp eq i8 %0, %1
%spec.select.i = and i1 %.not.i, %4
br label %"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit"
"_ZN55_$LT$T$u20$as$u20$core..option..SpecOptionPartialEq$GT$2eq17hb7e7beacecde585fE.exit": ; preds = %bb1.i, %bb3.i
%.0.i = phi i1 [ %3, %bb1.i ], [ %spec.select.i, %bb3.i ]
ret i1 %.0.i
}
```
After:
```llvm
; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone willreturn uwtable
define noundef zeroext i1 `@ordering_eq(i8` noundef %0, i8 noundef %1) unnamed_addr #1 {
start:
%2 = icmp eq i8 %0, %1
ret i1 %2
}
```
(Which <https://alive2.llvm.org/ce/z/-rop5r> says LLVM *could* just do itself, but there's probably an issue already open for that problem from when this was originally looked at for `Option<NonZeroU8>` and friends.)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a conservative approach to the introduction of
DropIf.In some cases, we still resort to drop flags and branches in the CFG.
Those do not break the promises we make about
DropIfbut makes it less effectiveAt the moment these include:
We also need to de-elaborate drops for async generators. For reasons that are still not clear to me, we run drop elaboration twice on those, which was previously possible but now isn't since domain and codomain of the drop elaboration 'function' are now different (dropifinit -> dropif)