[llvm] r336965 - [InstCombine] return when SimplifyAssociativeOrCommutative makes a change

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 12 18:18:07 PDT 2018


Author: spatel
Date: Thu Jul 12 18:18:07 2018
New Revision: 336965

URL: http://llvm.org/viewvc/llvm-project?rev=336965&view=rev
Log:
[InstCombine] return when SimplifyAssociativeOrCommutative makes a change

This bug was created by rL335258 because we used to always call instsimplify
after trying the associative folds. After that change it became possible
for subsequent folds to encounter unsimplified code (and potentially assert
because of it). 

Instead of carrying changed state through instcombine, we can just return 
immediately. This allows instsimplify to run, so we can continue assuming
that easy folds have already occurred.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
    llvm/trunk/test/Transforms/InstCombine/and-or-icmps.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=336965&r1=336964&r2=336965&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Thu Jul 12 18:18:07 2018
@@ -1126,7 +1126,9 @@ Instruction *InstCombiner::visitAdd(Bina
                                  SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -1367,6 +1369,7 @@ Instruction *InstCombiner::visitAdd(Bina
   // TODO(jingyue): Consider willNotOverflowSignedAdd and
   // willNotOverflowUnsignedAdd to reduce the number of invocations of
   // computeKnownBits.
+  bool Changed = false;
   if (!I.hasNoSignedWrap() && willNotOverflowSignedAdd(LHS, RHS, I)) {
     Changed = true;
     I.setHasNoSignedWrap(true);
@@ -1388,7 +1391,9 @@ Instruction *InstCombiner::visitFAdd(Bin
                                   SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -1471,7 +1476,7 @@ Instruction *InstCombiner::visitFAdd(Bin
       return replaceInstUsesWith(I, V);
   }
 
-  return Changed ? &I : nullptr;
+  return nullptr;
 }
 
 /// Optimize pointer differences into the same array into a size.  Consider:

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=336965&r1=336964&r2=336965&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Thu Jul 12 18:18:07 2018
@@ -1405,7 +1405,9 @@ Instruction *InstCombiner::visitAnd(Bina
                                  SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -1648,7 +1650,7 @@ Instruction *InstCombiner::visitAnd(Bina
       A->getType()->isIntOrIntVectorTy(1))
     return SelectInst::Create(A, Op0, Constant::getNullValue(I.getType()));
 
-  return Changed ? &I : nullptr;
+  return nullptr;
 }
 
 /// Given an OR instruction, check to see if this is a bswap idiom. If so,
@@ -2020,7 +2022,9 @@ Instruction *InstCombiner::visitOr(Binar
                                 SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -2287,7 +2291,7 @@ Instruction *InstCombiner::visitOr(Binar
     }
   }
 
-  return Changed ? &I : nullptr;
+  return nullptr;
 }
 
 /// A ^ B can be specified using other logic ops in a variety of patterns. We
@@ -2474,7 +2478,9 @@ Instruction *InstCombiner::visitXor(Bina
                                  SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -2785,5 +2791,5 @@ Instruction *InstCombiner::visitXor(Bina
     }
   }
 
-  return Changed ? &I : nullptr;
+  return nullptr;
 }

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=336965&r1=336964&r2=336965&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Thu Jul 12 18:18:07 2018
@@ -130,7 +130,9 @@ Instruction *InstCombiner::visitMul(Bina
                                  SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -393,6 +395,7 @@ Instruction *InstCombiner::visitMul(Bina
     }
   }
 
+  bool Changed = false;
   if (!I.hasNoSignedWrap() && willNotOverflowSignedMul(Op0, Op1, I)) {
     Changed = true;
     I.setHasNoSignedWrap(true);
@@ -412,7 +415,9 @@ Instruction *InstCombiner::visitFMul(Bin
                                   SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  bool Changed = SimplifyAssociativeOrCommutative(I);
+  if (SimplifyAssociativeOrCommutative(I))
+    return &I;
+
   if (Instruction *X = foldShuffledBinop(I))
     return X;
 
@@ -542,7 +547,7 @@ Instruction *InstCombiner::visitFMul(Bin
     }
   }
 
-  return Changed ? &I : nullptr;
+  return nullptr;
 }
 
 /// Fold a divide or remainder with a select instruction divisor when one of the

Modified: llvm/trunk/test/Transforms/InstCombine/and-or-icmps.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/and-or-icmps.ll?rev=336965&r1=336964&r2=336965&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/and-or-icmps.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/and-or-icmps.ll Thu Jul 12 18:18:07 2018
@@ -199,3 +199,57 @@ define <2 x i1> @and_ne_with_diff_one_sp
   ret <2 x i1> %and
 }
 
+; This is a fuzzer-generated test that would assert because
+; we'd get into foldAndOfICmps() without running InstSimplify
+; on an 'and' that should have been killed. It's not obvious
+; why, but removing anything hides the bug, hence the long test.
+
+define void @simplify_before_foldAndOfICmps() {
+; CHECK-LABEL: @simplify_before_foldAndOfICmps(
+; CHECK-NEXT:    [[A8:%.*]] = alloca i16, align 2
+; CHECK-NEXT:    [[L7:%.*]] = load i16, i16* [[A8]], align 2
+; CHECK-NEXT:    [[C10:%.*]] = icmp ult i16 [[L7]], 2
+; CHECK-NEXT:    [[C7:%.*]] = icmp slt i16 [[L7]], 0
+; CHECK-NEXT:    [[C18:%.*]] = or i1 [[C7]], [[C10]]
+; CHECK-NEXT:    [[L7_LOBIT:%.*]] = ashr i16 [[L7]], 15
+; CHECK-NEXT:    [[TMP1:%.*]] = sext i16 [[L7_LOBIT]] to i64
+; CHECK-NEXT:    [[G26:%.*]] = getelementptr i1, i1* null, i64 [[TMP1]]
+; CHECK-NEXT:    store i16 [[L7]], i16* undef, align 2
+; CHECK-NEXT:    store i1 [[C18]], i1* undef, align 1
+; CHECK-NEXT:    store i1* [[G26]], i1** undef, align 8
+; CHECK-NEXT:    ret void
+;
+  %A8 = alloca i16
+  %L7 = load i16, i16* %A8
+  %G21 = getelementptr i16, i16* %A8, i8 -1
+  %B11 = udiv i16 %L7, -1
+  %G4 = getelementptr i16, i16* %A8, i16 %B11
+  %L2 = load i16, i16* %G4
+  %L = load i16, i16* %G4
+  %B23 = mul i16 %B11, %B11
+  %L4 = load i16, i16* %A8
+  %B21 = sdiv i16 %L7, %L4
+  %B7 = sub i16 0, %B21
+  %B18 = mul i16 %B23, %B7
+  %C10 = icmp ugt i16 %L, %B11
+  %B20 = and i16 %L7, %L2
+  %B1 = mul i1 %C10, true
+  %C5 = icmp sle i16 %B21, %L
+  %C11 = icmp ule i16 %B21, %L
+  %C7 = icmp slt i16 %B20, 0
+  %B29 = srem i16 %L4, %B18
+  %B15 = add i1 %C7, %C10
+  %B19 = add i1 %C11, %B15
+  %C6 = icmp sge i1 %C11, %B19
+  %B33 = or i16 %B29, %L4
+  %C13 = icmp uge i1 %C5, %B1
+  %C3 = icmp ult i1 %C13, %C6
+  store i16 undef, i16* %G21
+  %C18 = icmp ule i1 %C10, %C7
+  %G26 = getelementptr i1, i1* null, i1 %C3
+  store i16 %B33, i16* undef
+  store i1 %C18, i1* undef
+  store i1* %G26, i1** undef
+  ret void
+}
+




More information about the llvm-commits mailing list