[PATCH] D49179: [InstCombine] Fold x & (-1 >> y) == x to x u<= (-1 >> y)

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 12 08:01:30 PDT 2018


lebedev.ri added inline comments.


================
Comment at: llvm/trunk/test/Transforms/InstCombine/icmp-logical.ll:91
 ; CHECK-LABEL: @masked_or_A(
-; CHECK-NEXT:    [[MASK2:%.*]] = and i32 [[A:%.*]], 39
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp ult i32 [[A:%.*]], 8
+; CHECK-NEXT:    [[MASK2:%.*]] = and i32 [[A]], 39
----------------
lebedev.ri wrote:
> yamauchi wrote:
> > It seems like the simplification of this change (D49179) triggers before this original simplification triggers and the original simplification no longer triggers? My guess is that this test just means to test a plain or-case and it may make sense to use some other values like 14 and 78 (shifted left by 1 bit, instead of 7 and 39) and preserve the original intention of the test.
> > 
> > Note the next test @masked_or_A_slightly_optimized has the same code as the after-simplification code of this function. Not sure if it is a just a coincidence.
> > 
> > and the original simplification no longer triggers?
> In any case, the original simplification clearly does not handle this pattern.
> 
> > My guess is that this test just means to test a plain or-case and it may make sense to use some other values like 14 and 78 (shifted left by 1 bit, instead of 7 and 39) and preserve the original intention of the test.
> 
> Hmm, thanks, might be a good idea..
> 
> > Note the next test @masked_or_A_slightly_optimized has the same code as the after-simplification code of this function. Not sure if it is a just a coincidence.
> 
> I have added `@masked_or_A_slightly_optimized` in rL336784 after noticing this regression, to have a test regardless.
@yamauchi committed in rL336912, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D49179





More information about the llvm-commits mailing list