[llvm] [InstCombine] Resolve FIXME: Use commutative matchers (PR #81060)

via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 7 16:34:28 PST 2024


https://github.com/AtariDreams updated https://github.com/llvm/llvm-project/pull/81060

>From bcb18c6eb58c27167d657ff2afd8e4e534e1a9ec Mon Sep 17 00:00:00 2001
From: Rose <83477269+AtariDreams at users.noreply.github.com>
Date: Wed, 7 Feb 2024 18:53:04 -0500
Subject: [PATCH] [InstCombine] Resolve FIXME: Use commutative matchers

---
 .../InstCombine/InstCombineAddSub.cpp         |  20 +--
 .../InstCombine/InstCombineAndOrXor.cpp       | 115 +++++++++---------
 2 files changed, 65 insertions(+), 70 deletions(-)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index cfff5df9ff5005..6e84740eab0c8c 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -1531,8 +1531,8 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
   {
     // (A + C1) + (C2 - B) --> (A - B) + (C1 + C2)
     Constant *C1, *C2;
-    if (match(&I, m_c_Add(m_Add(m_Value(A), m_ImmConstant(C1)),
-                          m_Sub(m_ImmConstant(C2), m_Value(B)))) &&
+    if (match(&I, m_Add(m_c_Add(m_Value(A), m_ImmConstant(C1)),
+                        m_Sub(m_ImmConstant(C2), m_Value(B)))) &&
         (LHS->hasOneUse() || RHS->hasOneUse())) {
       Value *Sub = Builder.CreateSub(A, B);
       return BinaryOperator::CreateAdd(Sub, ConstantExpr::getAdd(C1, C2));
@@ -1596,7 +1596,7 @@ Instruction *InstCombinerImpl::visitAdd(BinaryOperator &I) {
 
   // (add (or A, B) (and A, B)) --> (add A, B)
   // (add (and A, B) (or A, B)) --> (add A, B)
-  if (match(&I, m_c_BinOp(m_Or(m_Value(A), m_Value(B)),
+  if (match(&I, m_c_BinOp(m_c_Or(m_Value(A), m_Value(B)),
                           m_c_And(m_Deferred(A), m_Deferred(B))))) {
     // Replacing operands in-place to preserve nuw/nsw flags.
     replaceOperand(I, 0, A);
@@ -2223,7 +2223,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
 
   // (X + -1) - Y --> ~Y + X
   Value *X, *Y;
-  if (match(Op0, m_OneUse(m_Add(m_Value(X), m_AllOnes()))))
+  if (match(Op0, m_OneUse(m_c_Add(m_Value(X), m_AllOnes()))))
     return BinaryOperator::CreateAdd(Builder.CreateNot(Op1), X);
 
   // Reassociate sub/add sequences to create more add instructions and
@@ -2255,7 +2255,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
     // This is done in other passes, but we want to be able to consume this
     // pattern in InstCombine so we can generate it without creating infinite
     // loops.
-    if (match(Op0, m_Add(m_Value(X), m_Value(Z))) &&
+    if (match(Op0, m_c_Add(m_Value(X), m_Value(Z))) &&
         match(Op1, m_c_Add(m_Value(Y), m_Specific(Z))))
       return BinaryOperator::CreateSub(X, Y);
 
@@ -2345,7 +2345,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
     // C2 is negative pow2 || sub nuw
     const APInt *C2, *C3;
     BinaryOperator *InnerSub;
-    if (match(Op1, m_OneUse(m_And(m_BinOp(InnerSub), m_APInt(C2)))) &&
+    if (match(Op1, m_OneUse(m_c_And(m_BinOp(InnerSub), m_APInt(C2)))) &&
         match(InnerSub, m_Sub(m_APInt(C3), m_Value(X))) &&
         (InnerSub->hasNoUnsignedWrap() || C2->isNegatedPowerOf2())) {
       APInt C2AndC3 = *C2 & *C3;
@@ -2382,7 +2382,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   // (sub (add A, B) (or A, B)) --> (and A, B)
   {
     Value *A, *B;
-    if (match(Op0, m_Add(m_Value(A), m_Value(B))) &&
+    if (match(Op0, m_And(m_Value(A), m_Value(B))) &&
         match(Op1, m_c_Or(m_Specific(A), m_Specific(B))))
       return BinaryOperator::CreateAnd(A, B);
   }
@@ -2390,7 +2390,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   // (sub (add A, B) (and A, B)) --> (or A, B)
   {
     Value *A, *B;
-    if (match(Op0, m_Add(m_Value(A), m_Value(B))) &&
+    if (match(Op0, m_And(m_Value(A), m_Value(B))) &&
         match(Op1, m_c_And(m_Specific(A), m_Specific(B))))
       return BinaryOperator::CreateOr(A, B);
   }
@@ -2424,7 +2424,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   {
     Value *Y;
     // ((X | Y) - X) --> (~X & Y)
-    if (match(Op0, m_OneUse(m_c_Or(m_Value(Y), m_Specific(Op1)))))
+    if (match(Op0, m_OneUse(m_Or(m_Value(Y), m_Specific(Op1)))))
       return BinaryOperator::CreateAnd(
           Y, Builder.CreateNot(Op1, Op1->getName() + ".not"));
   }
@@ -2442,7 +2442,7 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   {
     // (sub (and Op1, C), Op1) --> neg (and Op1, ~C)
     Constant *C;
-    if (match(Op0, m_OneUse(m_And(m_Specific(Op1), m_Constant(C))))) {
+    if (match(Op0, m_OneUse(m_c_And(m_Specific(Op1), m_Constant(C))))) {
       return BinaryOperator::CreateNeg(
           Builder.CreateAnd(Op1, Builder.CreateNot(C)));
     }
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index aa3b9da924aa0b..e908a048f00689 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -2260,9 +2260,6 @@ static Value *simplifyAndOrWithOpReplaced(Value *X, Value *Y, bool IsAnd,
   return nullptr;
 }
 
-// FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
-// here. We should standardize that construct where it is needed or choose some
-// other way to ensure that commutated variants of patterns are not missed.
 Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
   Type *Ty = I.getType();
 
@@ -2447,8 +2444,8 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
     // This is intentionally placed after the narrowing transforms for
     // efficiency (transform directly to the narrow logic op if possible).
     // If the mask is only needed on one incoming arm, push the 'and' op up.
-    if (match(Op0, m_OneUse(m_Xor(m_Value(X), m_Value(Y)))) ||
-        match(Op0, m_OneUse(m_Or(m_Value(X), m_Value(Y))))) {
+    if (match(Op0, m_OneUse(m_c_Xor(m_Value(X), m_Value(Y)))) ||
+        match(Op0, m_OneUse(m_c_Or(m_Value(X), m_Value(Y))))) {
       APInt NotAndMask(~(*C));
       BinaryOperator::BinaryOps BinOp = cast<BinaryOperator>(Op0)->getOpcode();
       if (MaskedValueIsZero(X, NotAndMask, 0, &I)) {
@@ -2542,8 +2539,8 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
     }
   }
 
-  if (match(&I, m_And(m_OneUse(m_Shl(m_ZExt(m_Value(X)), m_Value(Y))),
-                      m_SignMask())) &&
+  if (match(&I, m_c_And(m_OneUse(m_Shl(m_ZExt(m_Value(X)), m_Value(Y))),
+                        m_SignMask())) &&
       match(Y, m_SpecificInt_ICMP(
                    ICmpInst::Predicate::ICMP_EQ,
                    APInt(Ty->getScalarSizeInBits(),
@@ -2593,8 +2590,9 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
       return BinaryOperator::CreateAnd(Op1, B);
 
     // (A ^ B) & ((B ^ C) ^ A) -> (A ^ B) & ~C
-    if (match(Op0, m_Xor(m_Value(A), m_Value(B))) &&
-        match(Op1, m_Xor(m_Xor(m_Specific(B), m_Value(C)), m_Specific(A)))) {
+    if (match(Op0, m_c_Xor(m_Value(A), m_Value(B))) &&
+        match(Op1,
+              m_c_Xor(m_c_Xor(m_Specific(B), m_Value(C)), m_Specific(A)))) {
       Value *NotC = Op1->hasOneUse()
                         ? Builder.CreateNot(C)
                         : getFreelyInverted(C, C->hasOneUse(), &Builder);
@@ -2603,8 +2601,8 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
     }
 
     // ((A ^ C) ^ B) & (B ^ A) -> (B ^ A) & ~C
-    if (match(Op0, m_Xor(m_Xor(m_Value(A), m_Value(C)), m_Value(B))) &&
-        match(Op1, m_Xor(m_Specific(B), m_Specific(A)))) {
+    if (match(Op0, m_c_Xor(m_c_Xor(m_Value(A), m_Value(C)), m_Value(B))) &&
+        match(Op1, m_c_Xor(m_Specific(B), m_Specific(A)))) {
       Value *NotC = Op0->hasOneUse()
                         ? Builder.CreateNot(C)
                         : getFreelyInverted(C, C->hasOneUse(), &Builder);
@@ -2727,8 +2725,9 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
                               Constant::getNullValue(Ty));
 
   // (-1 + A) & B --> A ? 0 : B where A is 0/1.
-  if (match(&I, m_c_And(m_OneUse(m_Add(m_ZExtOrSelf(m_Value(A)), m_AllOnes())),
-                        m_Value(B)))) {
+  if (match(&I,
+            m_c_And(m_OneUse(m_c_Add(m_ZExtOrSelf(m_Value(A)), m_AllOnes())),
+                    m_Value(B)))) {
     if (A->getType()->isIntOrIntVectorTy(1))
       return SelectInst::Create(A, Constant::getNullValue(Ty), B);
     if (computeKnownBits(A, /* Depth */ 0, &I).countMaxActiveBits() <= 1) {
@@ -2883,24 +2882,24 @@ static Instruction *matchFunnelShift(Instruction &Or, InstCombinerImpl &IC,
       // (shl ShVal, (X & (Width - 1))) | (lshr ShVal, ((-X) & (Width - 1)))
       Value *X;
       unsigned Mask = Width - 1;
-      if (match(L, m_And(m_Value(X), m_SpecificInt(Mask))) &&
-          match(R, m_And(m_Neg(m_Specific(X)), m_SpecificInt(Mask))))
+      if (match(L, m_c_And(m_Value(X), m_SpecificInt(Mask))) &&
+          match(R, m_c_And(m_Neg(m_Specific(X)), m_SpecificInt(Mask))))
         return X;
 
       // (shl ShVal, X) | (lshr ShVal, ((-X) & (Width - 1)))
-      if (match(R, m_And(m_Neg(m_Specific(L)), m_SpecificInt(Mask))))
+      if (match(R, m_c_And(m_Neg(m_Specific(L)), m_SpecificInt(Mask))))
         return L;
 
       // Similar to above, but the shift amount may be extended after masking,
       // so return the extended value as the parameter for the intrinsic.
-      if (match(L, m_ZExt(m_And(m_Value(X), m_SpecificInt(Mask)))) &&
-          match(R,
-                m_And(m_Neg(m_ZExt(m_And(m_Specific(X), m_SpecificInt(Mask)))),
-                      m_SpecificInt(Mask))))
+      if (match(L, m_ZExt(m_c_And(m_Value(X), m_SpecificInt(Mask)))) &&
+          match(R, m_And(m_Neg(m_ZExt(
+                             m_c_And(m_Specific(X), m_SpecificInt(Mask)))),
+                         m_SpecificInt(Mask))))
         return L;
 
-      if (match(L, m_ZExt(m_And(m_Value(X), m_SpecificInt(Mask)))) &&
-          match(R, m_ZExt(m_And(m_Neg(m_Specific(X)), m_SpecificInt(Mask)))))
+      if (match(L, m_ZExt(m_c_And(m_Value(X), m_SpecificInt(Mask)))) &&
+          match(R, m_ZExt(m_c_And(m_Neg(m_Specific(X)), m_SpecificInt(Mask)))))
         return L;
 
       return nullptr;
@@ -3416,9 +3415,6 @@ Value *InstCombinerImpl::foldAndOrOfICmps(ICmpInst *LHS, ICmpInst *RHS,
   return foldAndOrOfICmpsUsingRanges(LHS, RHS, IsAnd);
 }
 
-// FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
-// here. We should standardize that construct where it is needed or choose some
-// other way to ensure that commutated variants of patterns are not missed.
 Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   if (Value *V = simplifyOrInst(I.getOperand(0), I.getOperand(1),
                                 SQ.getWithInstruction(&I)))
@@ -3485,7 +3481,8 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
 
   Value *X, *Y;
   const APInt *CV;
-  if (match(&I, m_c_Or(m_OneUse(m_Xor(m_Value(X), m_APInt(CV))), m_Value(Y))) &&
+  if (match(&I,
+            m_c_Or(m_OneUse(m_c_Xor(m_Value(X), m_APInt(CV))), m_Value(Y))) &&
       !CV->isAllOnes() && MaskedValueIsZero(Y, *CV, 0, &I)) {
     // (X ^ C) | Y -> (X | Y) ^ C iff Y & C == 0
     // The check for a 'not' op is for efficiency (if Y is known zero --> ~X).
@@ -3495,7 +3492,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
 
   // If the operands have no common bits set:
   // or (mul X, Y), X --> add (mul X, Y), X --> mul X, (Y + 1)
-  if (match(&I, m_c_DisjointOr(m_OneUse(m_Mul(m_Value(X), m_Value(Y))),
+  if (match(&I, m_c_DisjointOr(m_OneUse(m_c_Mul(m_Value(X), m_Value(Y))),
                                m_Deferred(X)))) {
     Value *IncrementY = Builder.CreateAdd(Y, ConstantInt::get(Ty, 1));
     return BinaryOperator::CreateMul(X, IncrementY);
@@ -3593,13 +3590,13 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   }
 
   // (A ^ B) | ((B ^ C) ^ A) -> (A ^ B) | C
-  if (match(Op0, m_Xor(m_Value(A), m_Value(B))))
-    if (match(Op1, m_Xor(m_Xor(m_Specific(B), m_Value(C)), m_Specific(A))))
+  if (match(Op0, m_c_Xor(m_Value(A), m_Value(B))))
+    if (match(Op1, m_c_Xor(m_c_Xor(m_Specific(B), m_Value(C)), m_Specific(A))))
       return BinaryOperator::CreateOr(Op0, C);
 
   // ((A ^ C) ^ B) | (B ^ A) -> (B ^ A) | C
-  if (match(Op0, m_Xor(m_Xor(m_Value(A), m_Value(C)), m_Value(B))))
-    if (match(Op1, m_Xor(m_Specific(B), m_Specific(A))))
+  if (match(Op0, m_c_Xor(m_c_Xor(m_Value(A), m_Value(C)), m_Value(B))))
+    if (match(Op1, m_c_Xor(m_Specific(B), m_Specific(A))))
       return BinaryOperator::CreateOr(Op1, C);
 
   // ((A & B) ^ C) | B -> C | B
@@ -3615,12 +3612,12 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
 
   // Canonicalize xor to the RHS.
   bool SwappedForXor = false;
-  if (match(Op0, m_Xor(m_Value(), m_Value()))) {
+  if (match(Op0, m_c_Xor(m_Value(), m_Value()))) {
     std::swap(Op0, Op1);
     SwappedForXor = true;
   }
 
-  if (match(Op1, m_Xor(m_Value(A), m_Value(B)))) {
+  if (match(Op1, m_c_Xor(m_Value(A), m_Value(B)))) {
     // (A | ?) | (A ^ B) --> (A | ?) | B
     // (B | ?) | (A ^ B) --> (B | ?) | A
     if (match(Op0, m_c_Or(m_Specific(A), m_Value())))
@@ -3630,8 +3627,8 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
 
     // (A & B) | (A ^ B) --> A | B
     // (B & A) | (A ^ B) --> A | B
-    if (match(Op0, m_And(m_Specific(A), m_Specific(B))) ||
-        match(Op0, m_And(m_Specific(B), m_Specific(A))))
+    if (match(Op0, m_c_And(m_Specific(A), m_Specific(B))) ||
+        match(Op0, m_c_And(m_Specific(B), m_Specific(A))))
       return BinaryOperator::CreateOr(A, B);
 
     // ~A | (A ^ B) --> ~(A & B)
@@ -3690,7 +3687,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
     // TODO: Make this recursive; it's a little tricky because an arbitrary
     // number of 'or' instructions might have to be created.
     Value *X, *Y;
-    if (LHS && match(Op1, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
+    if (LHS && match(Op1, m_OneUse(m_c_LogicalOr(m_Value(X), m_Value(Y))))) {
       bool IsLogical = isa<SelectInst>(Op1);
       // LHS | (X || Y) --> (LHS || X) || Y
       if (auto *Cmp = dyn_cast<ICmpInst>(X))
@@ -3707,7 +3704,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
                                             ? Builder.CreateLogicalOr(X, Res)
                                             : Builder.CreateOr(X, Res));
     }
-    if (RHS && match(Op0, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
+    if (RHS && match(Op0, m_OneUse(m_c_LogicalOr(m_Value(X), m_Value(Y))))) {
       bool IsLogical = isa<SelectInst>(Op0);
       // (X || Y) | RHS --> (X || RHS) || Y
       if (auto *Cmp = dyn_cast<ICmpInst>(X))
@@ -3756,7 +3753,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   // (X|C) | V --> (X|V) | C
   ConstantInt *CI;
   if (Op0->hasOneUse() && !match(Op1, m_ConstantInt()) &&
-      match(Op0, m_Or(m_Value(A), m_ConstantInt(CI)))) {
+      match(Op0, m_c_Or(m_Value(A), m_ConstantInt(CI)))) {
     Value *Inner = Builder.CreateOr(A, Op1);
     Inner->takeName(Op0);
     return BinaryOperator::CreateOr(Inner, CI);
@@ -3795,9 +3792,9 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
     // ((A & B) ^ B) | ((A & B) ^ A) -> A ^ B
     // (B ^ (A & B)) | (A ^ (A & B)) -> A ^ B
     const auto TryXorOpt = [&](Value *Lhs, Value *Rhs) -> Instruction * {
-      if (match(Lhs, m_c_Xor(m_And(m_Value(A), m_Value(B)), m_Deferred(A))) &&
-          match(Rhs,
-                m_c_Xor(m_And(m_Specific(A), m_Specific(B)), m_Deferred(B)))) {
+      if (match(Lhs, m_c_Xor(m_c_And(m_Value(A), m_Value(B)), m_Deferred(A))) &&
+          match(Rhs, m_c_Xor(m_c_And(m_Specific(A), m_Specific(B)),
+                             m_Deferred(B)))) {
         return BinaryOperator::CreateXor(A, B);
       }
       return nullptr;
@@ -3874,7 +3871,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
 
   // Improve "get low bit mask up to and including bit X" pattern:
   //   (1 << X) | ((1 << X) + -1)  -->  -1 l>> (bitwidth(x) - 1 - X)
-  if (match(&I, m_c_Or(m_Add(m_Shl(m_One(), m_Value(X)), m_AllOnes()),
+  if (match(&I, m_c_Or(m_c_Add(m_Shl(m_One(), m_Value(X)), m_AllOnes()),
                        m_Shl(m_One(), m_Deferred(X)))) &&
       match(&I, m_c_Or(m_OneUse(m_Value()), m_Value()))) {
     Value *Sub = Builder.CreateSub(
@@ -3955,7 +3952,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
   }
 
   // (X & C1) | C2 -> X & (C1 | C2) iff (X & C2) == C2
-  if (match(Op0, m_OneUse(m_And(m_Value(X), m_APInt(C1)))) &&
+  if (match(Op0, m_OneUse(m_c_And(m_Value(X), m_APInt(C1)))) &&
       match(Op1, m_APInt(C2))) {
     KnownBits KnownX = computeKnownBits(X, /*Depth*/ 0, &I);
     if ((KnownX.One & *C2) == *C2)
@@ -3988,7 +3985,7 @@ static Instruction *foldXorToXor(BinaryOperator &I,
   // (A & B) ^ (B | A) -> A ^ B
   // (A | B) ^ (A & B) -> A ^ B
   // (A | B) ^ (B & A) -> A ^ B
-  if (match(&I, m_c_Xor(m_And(m_Value(A), m_Value(B)),
+  if (match(&I, m_c_Xor(m_c_And(m_Value(A), m_Value(B)),
                         m_c_Or(m_Deferred(A), m_Deferred(B)))))
     return BinaryOperator::CreateXor(A, B);
 
@@ -4017,9 +4014,9 @@ static Instruction *foldXorToXor(BinaryOperator &I,
   // (A & B) ^ ~(A | B) -> ~(A ^ B)
   // (A & B) ^ ~(B | A) -> ~(A ^ B)
   // Complexity sorting ensures the not will be on the right side.
-  if ((match(Op0, m_Or(m_Value(A), m_Value(B))) &&
+  if ((match(Op0, m_c_Or(m_Value(A), m_Value(B))) &&
        match(Op1, m_Not(m_c_And(m_Specific(A), m_Specific(B))))) ||
-      (match(Op0, m_And(m_Value(A), m_Value(B))) &&
+      (match(Op0, m_c_And(m_Value(A), m_Value(B))) &&
        match(Op1, m_Not(m_c_Or(m_Specific(A), m_Specific(B))))))
     return BinaryOperator::CreateNot(Builder.CreateXor(A, B));
 
@@ -4402,7 +4399,7 @@ Instruction *InstCombinerImpl::foldNot(BinaryOperator &I) {
     Value *NotY = Builder.CreateNot(Y, Y->getName() + ".not");
     return BinaryOperator::CreateAnd(X, NotY);
   }
-  if (match(NotOp, m_OneUse(m_LogicalOr(m_Not(m_Value(X)), m_Value(Y))))) {
+  if (match(NotOp, m_OneUse(m_c_LogicalOr(m_Not(m_Value(X)), m_Value(Y))))) {
     Value *NotY = Builder.CreateNot(Y, Y->getName() + ".not");
     return SelectInst::Create(X, NotY, ConstantInt::getFalse(Ty));
   }
@@ -4564,9 +4561,6 @@ Instruction *InstCombinerImpl::foldNot(BinaryOperator &I) {
   return nullptr;
 }
 
-// FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
-// here. We should standardize that construct where it is needed or choose some
-// other way to ensure that commutated variants of patterns are not missed.
 Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
   if (Value *V = simplifyXorInst(I.getOperand(0), I.getOperand(1),
                                  SQ.getWithInstruction(&I)))
@@ -4617,7 +4611,7 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
   if (match(Op1, m_Constant(C1))) {
     Constant *C2;
 
-    if (match(Op0, m_OneUse(m_Or(m_Value(X), m_ImmConstant(C2)))) &&
+    if (match(Op0, m_OneUse(m_c_Or(m_Value(X), m_ImmConstant(C2)))) &&
         match(C1, m_ImmConstant())) {
       // (X | C2) ^ C1 --> (X & ~C2) ^ (C1^C2)
       C2 = Constant::replaceUndefsWith(
@@ -4629,12 +4623,12 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
     }
 
     // Use DeMorgan and reassociation to eliminate a 'not' op.
-    if (match(Op0, m_OneUse(m_Or(m_Not(m_Value(X)), m_Constant(C2))))) {
+    if (match(Op0, m_OneUse(m_c_Or(m_Not(m_Value(X)), m_Constant(C2))))) {
       // (~X | C2) ^ C1 --> ((X & ~C2) ^ -1) ^ C1 --> (X & ~C2) ^ ~C1
       Value *And = Builder.CreateAnd(X, ConstantExpr::getNot(C2));
       return BinaryOperator::CreateXor(And, ConstantExpr::getNot(C1));
     }
-    if (match(Op0, m_OneUse(m_And(m_Not(m_Value(X)), m_Constant(C2))))) {
+    if (match(Op0, m_OneUse(m_c_And(m_Not(m_Value(X)), m_Constant(C2))))) {
       // (~X & C2) ^ C1 --> ((X | ~C2) ^ -1) ^ C1 --> (X | ~C2) ^ ~C1
       Value *Or = Builder.CreateOr(X, ConstantExpr::getNot(C2));
       return BinaryOperator::CreateXor(Or, ConstantExpr::getNot(C1));
@@ -4775,10 +4769,10 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
 
   Value *A, *B, *C;
   // (A ^ B) ^ (A | C) --> (~A & C) ^ B -- There are 4 commuted variants.
-  if (match(&I, m_c_Xor(m_OneUse(m_Xor(m_Value(A), m_Value(B))),
+  if (match(&I, m_c_Xor(m_OneUse(m_c_Xor(m_Value(A), m_Value(B))),
                         m_OneUse(m_c_Or(m_Deferred(A), m_Value(C))))))
-      return BinaryOperator::CreateXor(
-          Builder.CreateAnd(Builder.CreateNot(A), C), B);
+    return BinaryOperator::CreateXor(Builder.CreateAnd(Builder.CreateNot(A), C),
+                                     B);
 
   // (A ^ B) ^ (B | C) --> (~B & C) ^ A -- There are 4 commuted variants.
   if (match(&I, m_c_Xor(m_OneUse(m_Xor(m_Value(A), m_Value(B))),
@@ -4860,10 +4854,11 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
   //   (X ^ C) ^ Y --> (X ^ Y) ^ C
   // Just like we do in other places, we completely avoid the fold
   // for constantexprs, at least to avoid endless combine loop.
-  if (match(&I, m_c_Xor(m_OneUse(m_Xor(m_CombineAnd(m_Value(X),
-                                                    m_Unless(m_ConstantExpr())),
-                                       m_ImmConstant(C1))),
-                        m_Value(Y))))
+  if (match(&I,
+            m_c_Xor(m_OneUse(m_c_Xor(
+                        m_CombineAnd(m_Value(X), m_Unless(m_ConstantExpr())),
+                        m_ImmConstant(C1))),
+                    m_Value(Y))))
     return BinaryOperator::CreateXor(Builder.CreateXor(X, Y), C1);
 
   if (Instruction *R = reassociateForUses(I, Builder))



More information about the llvm-commits mailing list