[PATCH] D126617: [InstCombine] Optimize shl+lshr+and conversion pattern

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1899
+      unsigned LShrEqBits = Log2C + C2->getZExtValue();
+      if (LShrEqBits < Width && LShrEqBits >= Log2C1) {
+        // iff C,C1 is pow2 and cttz(C1) < cttz(C)+C2 < BitWidth:
----------------
bcl5980 wrote:
> spatel wrote:
> > Do we really need both conditions? I removed one assumption, and it still shows as correct:
> > https://alive2.llvm.org/ce/z/nUAXL9
> Yeah, we can remove the condition cttz(C1) < cttz(C)+C2.
> Actually if cttz(C1) >= cttz(C)+C2, it will fall into D127122. 
> So still the question which pattern we should use by default, 'shift+and' or 'icmp+select'?
> 'shift+and' pattern can remain the information x is limited by bit width.
> 'icmp+select' can help to handle shift+and+xor case, and icmp can handle lshr, shl at the same time.
> For now what I do is keep 'shift+and' ASAP but if we prefer icmp+select I can remove the condition.
> Yeah, we can remove the condition cttz(C1) < cttz(C)+C2.
> Actually if cttz(C1) >= cttz(C)+C2, it will fall into D127122. 
Sorry for the wrong comments, actually if cttz(C1) >= cttz(C)+C2 it will always return 0.
Last comment works when C2 <= cttz(C1) < cttz(C)+C2.
I will remove cttz(C1) < cttz(C)+C2 to make the code easier.


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

https://reviews.llvm.org/D126617