[PATCH] D47428: [InstCombine] PR37603: low bit mask canonicalization

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/set-lowbits-mask-canonicalize.ll:204-211
 define i32 @shl_const(i32 %NBits) {
 ; CHECK-LABEL: @shl_const(
 ; CHECK-NEXT:    ret i32 131071
 ;
   %setbit = shl i32 1, 17
   %ret = add i32 %setbit, -1
   ret i32 %ret
----------------
I didn't see this test initially - it's ok to add it here, but it's better to include under test/Transforms/InstSimplify instead (if there's nothing like it already there).

But this doesn't exercise the case that I was thinking of. We should always constant fold simple constants like this. It's *constant expressions* that we (or at least me...multiple times!) forget and then result in fuzzer failures some time later. So the test will need a global variable or some other magic, but now that the code is dyn_cast'ing, it's probably a moot point.


Repository:
  rL LLVM

https://reviews.llvm.org/D47428