[PATCH] D43835: Simplify more cases of logical ops of masked icmps.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:440
+
+/// Try to fold (icmp(A & B) ==/!= 0) &/| (icmp(A & D) ==/!= E) into a single
+/// (icmp(A & X) ==/!= Y), where the left-hand side is of type Mask_NotAllZeros
----------------
icmp(A&B) ==/!= C


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:450
+  // We are given the canonical form:
+  //   (icmp ne (A & B), 0) & (icmp eq (A & D), E).
+  // where D & E == E.
----------------
Do you have test case cover this when LSH and RHS are swapped?

(icmp eq (A &D), E) & (icmp ne (A & B, 0))


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:454
+  // If IsAnd is false, we get it in negated form:
+  //   (icmp eq (A & B), 0) | (icmp ne (A & D), E) ->
+  //      !((icmp ne (A & B), 0) & (icmp eq (A & D), E)).
----------------
replace 0 with 'C' in the example.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:462
+    return nullptr;
+  ConstantInt *CCst = dyn_cast<ConstantInt>(C);
+  if (!CCst)
----------------
Do you need to check CCst is Zero?


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:474
+
+  // Update E to the canonical form when D is a power of two and RHS is
+  // canonicalized as,
----------------
It seems to me, it is sufficient to do the following check:

1) check if mask B is a subset of mask D
2) check if (E & B) != 0 is true 

If both 1) and 2) are satisfied, the simplification is legal.


Repository:
  rL LLVM

https://reviews.llvm.org/D43835