[InstSimplify] Use commutable matchers to shorten some code

[craig.topper]

Summary

This code was replicated two additional times to handle commuted cases, but I think a commutable matcher can take care of it.

[spatel]

Looks like good clean-up, but we only have 2 existing tests for these folds. There should be 4 commuted variants?

It's an independent change, but if we used m_APInt here, the code would be cleaner still and automatically work with splat vectors. You could make the additional commuted tests use vector types to serve double-duty if you want to add coverage for both of those changes.

Side note regarding commutation: just above here, simplifyAndOrOfICmps() calls simplifyPossiblyCastedAndOrOfICmps() with the params swapped to handle commutation, but some of the folds under there (ConstantRange checks at least) don't need to be commuted. Barring an inlining and analysis miracle, we're probably doing extra work for no reason on those, so untangling that would save compile-time.

[craig.topper]

So should I do the m_APInt change first so I can plug the commuted test hole with the vector tests?

[craig.topper]

Also should the matcher for the ands be commutable too?

[spatel]

In D33585#765865, @craig.topper wrote:

Also should the matcher for the ands be commutable too?

If we can make that work too, that would be great. :)
I don't have a preference about whether we change this in one step or more. Whatever seems easier to you. As long as we have at least a couple more tests, we should be sure that it all works as expected.

[craig.topper]

Rebase on top of the patch to use m_APInt

[spatel]

LGTM.

lib/Analysis/InstructionSimplify.cpp

static Value *SimplifyOrInst(Value *Op0, Value *Op1, const SimplifyQuery &Q,
   ConstantInt *C1, *C2;
   if (match(Op0, m_And(m_Value(A), m_ConstantInt(C1))) &&
       match(Op1, m_And(m_Value(B), m_ConstantInt(C2)))) {
     if (C1->getValue() == ~C2->getValue()) {
       // (A & C1)|(B & C2)
       // If we have: ((V + N) & C1) | (V & C2)
       // .. and C2 = ~C1 and C2 is 0+1+ and (N & C2) == 0
       // replace with V+N.
-      Value *V1, *V2;
+      Value *N;
       if (C2->getValue().isMask() && // C2 == 0+1+
-          match(A, m_Add(m_Value(V1), m_Value(V2)))) {
-        // Add commutes, try both ways.
-        if (V1 == B &&
-            MaskedValueIsZero(V2, C2->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
-          return A;
-        if (V2 == B &&
-            MaskedValueIsZero(V1, C2->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
+          match(A, m_c_Add(m_Specific(B), m_Value(N)))) {
+        if (MaskedValueIsZero(N, C2->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
           return A;
       }
       // Or commutes, try both ways.
       if (C1->getValue().isMask() &&
-          match(B, m_Add(m_Value(V1), m_Value(V2)))) {
+          match(B, m_c_Add(m_Specific(A), m_Value(N)))) {
         // Add commutes, try both ways.
-        if (V1 == A &&
-            MaskedValueIsZero(V2, C1->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
-          return B;
-        if (V2 == A &&
-            MaskedValueIsZero(V1, C1->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
+        if (MaskedValueIsZero(N, C1->getValue(), Q.DL, 0, Q.AC, Q.CxtI, Q.DT))
           return B;
       }
     }
   }
 
   // If the operation is with the result of a phi instruction, check whether
   // operating on all incoming values of the phi always yields the same value.
   if (isa<PHINode>(Op0) || isa<PHINode>(Op1))