[PATCH] D62818: [InstCombine] Introduce fold for icmp pred (and X, (sh signbit, Y)), 0.

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/onehot_merge.ll:18-23
+;CHECK:       [[TMP:%.*]] = shl i32 1, %c1
+;CHECK-NEXT:  [[TMP1:%.*]] = and i32 [[TMP]], %k
+;CHECK-NEXT:  icmp eq i32 [[TMP1]], 0
+;CHECK-NEXT:  [[TMP2:%.*]] = shl i32 %k, %c2
+;CHECK-NEXT:  icmp sgt i32 [[TMP2]], -1
+;CHECK-NEXT:  or i1
----------------
huihuiz wrote:
> spatel wrote:
> > lebedev.ri wrote:
> > > lebedev.ri wrote:
> > > > lebedev.ri wrote:
> > > > > Can you please regenerate the original test?
> > > > I'm not sure what's on the LHS of the diff, but ignoring the instruction count this looks like improvement to me.
> > > AH, you also want to str-replace `%tmp` with `%t`, it confuses the update script likely.
> > rL364546
> Actually we missed the fold for 
> 
> ```
> ((k & ( 1 l<< C1 )) == 0) || ((k & ( signbit l>> C2 )) == 0)
> -->
> ((k & (( 1 l<< C1 ) || ( signbit l>> C2 ))) != 0)
> ```
Thanks for the analysis!
@spatel does this fall into the nowadays reasoning that we shouldn't be doing too much folds into bitmath
here in instcombine? I'm almost tempted to say that this isn't a regression,
but the original fold that now no longer happens should be removed instead.



Repository:
  rL LLVM

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

https://reviews.llvm.org/D62818