[PATCH] D66232: [InstCombine] Try to reuse constant from select in leading comparison

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added a comment.

I like idea here and patch seems generally fine.

@nikic, @dmgreen, @spatel are probably interested to check this too.



================
Comment at: llvm/test/Transforms/InstCombine/unrecognized_three-way-comparison.ll:92-106
 define i32 @compare_against_two(i32 %x) {
 ; CHECK-LABEL: @compare_against_two(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[TMP0:%.*]] = icmp sgt i32 [[X:%.*]], 2
-; CHECK-NEXT:    br i1 [[TMP0]], label [[CALLFOO:%.*]], label [[EXIT:%.*]]
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[X:%.*]], 2
+; CHECK-NEXT:    [[CMP2_INV:%.*]] = icmp sgt i32 [[X]], 1
+; CHECK-NEXT:    [[SELECT1:%.*]] = select i1 [[CMP2_INV]], i32 1, i32 -1
+; CHECK-NEXT:    [[SELECT2:%.*]] = select i1 [[CMP1]], i32 0, i32 [[SELECT1]]
----------------
lebedev.ri wrote:
> xbolva00 wrote:
> > lebedev.ri wrote:
> > > This is clearly a regression.
> > > I suppose it means this fold happens before some other fold that used to happen,
> > > and that fold does not know how to deal with the new pattern.
> > > I'd prefer to look into this afterwards.
> > No status update here? Or did you find what is broken?
> > 
> > Patch looks fine I think, but this worries me -   we may realize later that the root cause of regression is possibly nontrivial thing which could lead us to rever this one. (and wasted time).
> Added some tests in rL369667.
> Whatever handles this is "simply" ignorant about commutativity.
Ok, thanks for info.

(Not a patch blocker)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66232/new/

https://reviews.llvm.org/D66232