[PATCH] D112108: [InstCombine] Fold `(a & ~b) & ~c` to `a & ~(b | c)`

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Please pre-commit the tests with current output, so we can confirm that we are testing all of the commuted patterns correctly.

This seems like another short-coming of the reassociation pass, but I think it's ok to deal with the minimal case here.
For example (if I'm seeing it correctly), this test still won't change:

  define i4 @src(i4 %a, i4 %b, i4 %c, i4 %d) {
    %notb = xor i4 %b, -1
    %notc = xor i4 %c, -1
    %and1 = and i4 %a, %notb
    %and2 = and i4 %and1, %d
    %and3 = and i4 %and2, %notc
    ret i4 %and3
  }

https://alive2.llvm.org/ce/z/_T8ZhP



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2024-2025
+
+    // ~C & (A & ~B) -> A & ~(B | C)
+    // ~C & (~B & A) -> A & ~(B | C)
+    if (match(Op1, m_c_And(m_Value(A), m_Not(m_Value(B)))))
----------------
I suspect this is dead code (so it can be removed) - complexity-based canonicalization should guarantee that an `and` is always op0 in this pattern.


================
Comment at: llvm/test/Transforms/InstCombine/and-xor-or.ll:531
+  %not2 = xor i32 %c, -1
+  %and1 = and i32 %a, %not1
+  %and2 = and i32 %and1, %not2
----------------
'not' is considered more complex than an argument, so this is not testing the pattern that you wanted to test.
See InstCombiner::getComplexity() for the details. Search for "thwart complexity-based canonicalization" in this test directory for test coverage that works around it.


================
Comment at: llvm/test/Transforms/InstCombine/and-xor-or.ll:578
+  %and1 = and i32 %a, %not1
+  %and2 = and i32 %not2, %and1
+  ret i32 %and2
----------------
Similar to above test comment - this is probably getting altered before we get to the new code in this patch, so it doesn't test what you expected.


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

https://reviews.llvm.org/D112108