[PATCH] D55745: [InstCombine] Simplify cttz/ctlz + icmp eq/ne into mask check

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 2 inline comments as done.
nikic added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:2796
+        : APInt::getOneBitSet(BitWidth, BitWidth - Num - 1);
+      Cmp.setOperand(0, Builder.CreateAnd(II->getArgOperand(0), Mask1));
+      Cmp.setOperand(1, ConstantInt::get(Ty, Mask2));
----------------
spatel wrote:
> We have to check hasOneUse() on the intrinsic, or we'll end up with extra instructions for an example like this:
> 
> ```
> define i1 @ctlz_eq_other_i32(i32 %x, i32* %p) {
>   %lz = tail call i32 @llvm.ctlz.i32(i32 %x, i1 false)
>   store i32 %lz, i32* %p
>   %cmp = icmp eq i32 %lz, 24
>   ret i1 %cmp
> }
> 
> ```
> 
> In cases where the 'and' gets removed, you don't need the one-use restriction, but I'm not sure if it's worth trying to differentiate those from the general case.
Thanks, I've added the hasOneUse() check and a couple tests. And yeah, I don't think it's worth handling those cases separately.


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

https://reviews.llvm.org/D55745