[PATCH] D25200: [InstCombine] New opportunities for FoldAndOfICmp and FoldXorOfICmp

[Ehsan Amiri via Phabricator via llvm-commits]

amehsan marked 7 inline comments as done.
amehsan added a comment.

Will shortly update the patch.



================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:755-758
+// For an ICMP where RHS is zero, we want to check if the ICMP is equivalent to
+// comparing a group of bits in an integer value against zero.
+BitGroupCheck isAnyBitSet(Value *LHS, ICmpInst::Predicate CC) {
+
----------------
spatel wrote:
> I've scanned over this a few times, and it just never reads clearly to me. Can you add comments and example patterns, so we'll know how this will be used?
added a comment. I hope that make is it more readable.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:850
+
+  if (RHSCst->isZero() && LHSCst->isZero()) {
+
----------------
spatel wrote:
> Can we check that RHSCst == LHSCst and remove the getBitWidth checks below?
See the respond to a similar comment below.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:947
+  // and (y = and b, mask2) where both masks have one bit and mask1 >= mask2
+  if (RHSCst->isZero() && LHSCst->isZero()) {
+    BitGroupCheck BGC1 = isAnyBitSet(Val, LHSCC);
----------------
spatel wrote:
> Can we check that RHSCst == LHSCst and remove the getBitWidth checks below?
getBitWidth is used for the mask which is operand of the and operator. RHSCst and LHSCst are operands of icmp instructions. I double checked the set of conditions that I have used below and they are all required. 


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:953-959
+      if (!BGC1.CheckIfSet &&
+          BGC1.Mask->countTrailingZeros() >=
+          BGC2.Mask->getBitWidth() - BGC2.Mask->countLeadingZeros() - 1)
+        return Builder->CreateICmp(ICmpInst::ICMP_ULT, Val, Val2);
+      else if (!BGC2.CheckIfSet &&
+          BGC2.Mask->countTrailingZeros() >=
+          BGC1.Mask->getBitWidth() - BGC1.Mask->countLeadingZeros() - 1)
----------------
spatel wrote:
> This needs an explanatory comment with an example.
Example is provided in lines 945-946. (line numbers may change after updating the review). Will revise the comment to be more general and contains more details.


https://reviews.llvm.org/D25200