[PATCH] D33172: [InstCombine] Simpify inverted predicates in 'or'

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2269-2270
 
+  // Change (or (X ? A : B), (!X ? C : D)) --> (X ? A : C), iff (B|D) == 0
+  // That is, if the comparison is based on the results of two select
+  // instructions, check whether those conditions are inverse icmp instructions
----------------
This comment didn't read clearly to me. The 'or' (not a comparison) is based on the selects.

Have the formula in the comment line up with the variable names in the code:
// or (select (cmp Pred1, A, B), C, 0), (select (cmp Pred2, A, B), D, 0) --> select (cmp Pred1, A, B), C, D
// when Pred1 is the inverse of Pred2.


================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2285-2290
+      auto XCmp = dyn_cast<CmpInst>(X);
+      auto YCmp = dyn_cast<CmpInst>(Y);
+      if (XCmp && YCmp &&
+          XCmp->getPredicate() == YCmp->getInversePredicate() &&
+          XCmp->getOperand(0) == YCmp->getOperand(0) &&
+          XCmp->getOperand(1) == YCmp->getOperand(1)) {
----------------
Use another pattern matcher here (warning: untested code):
  if (match(X, m_Cmp(Pred1, m_Value(A), m_Value(B))) &&
      match(Y, m_Cmp(Pred2, m_Specific(A), m_Specific(B))) &&
      CmpInst::getInversePredicate(Pred1) == Pred2)



================
Comment at: test/Transforms/InstCombine/logical-select.ll:67
+; Fold two selects with inverted predicates and zero operands.
+define i32 @pal(i32 %a, i32 %b, i32 %c, i32 %d) {
+; CHECK-LABEL: @pal(
----------------
We need more tests:
1. Check the case where the 0 operands are on the left instead of the right.
2. The code should work with fcmps, so we need a test for that.
3. The code should work with vectors, so we need a test for that.


https://reviews.llvm.org/D33172