[PATCH] D124119: [InstCombine] Combine instructions of type or/and where AND masks can be combined.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

This seems like a problem with the `-reassociate` pass - it seems to invert the form we want here.

However, I'm not opposed to another relatively simple reassociation transform here in instcombine (we have many already), but the patch as written is not general enough, missing commuted patterns/tests, and missing some kind of one-use check as noted earlier.

We could generalize to a canonicalization that pushes the 'and' values together, and then the optimization of combining mask constants will fall out from that:
https://alive2.llvm.org/ce/z/ZHgrvL

As code, that would be something like this:

  // (A & B) | (C | (A & D)) --> ((A & B) | (A & D)) | C
  if (match(Op0, m_And(m_Value(A), m_Value(B))) &&
      match(Op1, m_c_Or(m_Value(C), m_c_And(m_Specific(A), m_Value(D)))))
  ...

There are already 4 commuted possibilities there, but we'd also need to test for the pattern where "B" is the repeated operand on the right side:

  // (A & B) | (C | (B & D)) --> ((A & B) | (B & D)) | C

...and swap the operands of the final 'or'. So 16 total patterns to test for? If we make the matches require constants, we can reduce the number of possibilities (since we know that constants are always op1/right-side), but it's a less general transform.


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

https://reviews.llvm.org/D124119