[llvm-branch-commits] [llvm-branch] r168489 - in /llvm/branches/release_32: ./ lib/Transforms/Scalar/Reassociate.cpp test/Transforms/Reassociate/crash.ll

Pawel Wodnicki pawel at 32bitmicro.com
Wed Nov 21 22:21:31 PST 2012


Author: pawel
Date: Thu Nov 22 00:21:31 2012
New Revision: 168489

URL: http://llvm.org/viewvc/llvm-project?rev=168489&view=rev
Log:
Merging r168291: into the 3.2 release branch.

Fix PR14060, an infinite loop in reassociate.  The problem was that one of the
operands of the expression being written was wrongly thought to be reusable as
an inner node of the expression resulting in it turning up as both an inner node
*and* a leaf, creating a cycle in the def-use graph.  This would have caused the
verifier to blow up if things had gotten that far, however it managed to provoke
an infinite loop first.

Modified:
    llvm/branches/release_32/   (props changed)
    llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp
    llvm/branches/release_32/test/Transforms/Reassociate/crash.ll

Propchange: llvm/branches/release_32/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Nov 22 00:21:31 2012
@@ -1,3 +1,3 @@
 /llvm/branches/Apple/Pertwee:110850,110961
 /llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,167718-167719,167731,167737,167743,167750,167784,167811,167817,167855,167860-167864,167875,167942,167948,167966,168001,168035,168181,168189,168197-168198,168227,168280,168316,168319,168346,168352,168354,168361,168364
+/llvm/trunk:155241,167718-167719,167731,167737,167743,167750,167784,167811,167817,167855,167860-167864,167875,167942,167948,167966,168001,168035,168181,168189,168197-168198,168227,168280,168291,168316,168319,168346,168352,168354,168361,168364

Modified: llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp?rev=168489&r1=168488&r2=168489&view=diff
==============================================================================
--- llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp (original)
+++ llvm/branches/release_32/lib/Transforms/Scalar/Reassociate.cpp Thu Nov 22 00:21:31 2012
@@ -606,8 +606,8 @@
                                   SmallVectorImpl<ValueEntry> &Ops) {
   assert(Ops.size() > 1 && "Single values should be used directly!");
 
-  // Since our optimizations never increase the number of operations, the new
-  // expression can always be written by reusing the existing binary operators
+  // Since our optimizations should never increase the number of operations, the
+  // new expression can usually be written reusing the existing binary operators
   // from the original expression tree, without creating any new instructions,
   // though the rewritten expression may have a completely different topology.
   // We take care to not change anything if the new expression will be the same
@@ -621,6 +621,20 @@
   unsigned Opcode = I->getOpcode();
   BinaryOperator *Op = I;
 
+  /// NotRewritable - The operands being written will be the leaves of the new
+  /// expression and must not be used as inner nodes (via NodesToRewrite) by
+  /// mistake.  Inner nodes are always reassociable, and usually leaves are not
+  /// (if they were they would have been incorporated into the expression and so
+  /// would not be leaves), so most of the time there is no danger of this.  But
+  /// in rare cases a leaf may become reassociable if an optimization kills uses
+  /// of it, or it may momentarily become reassociable during rewriting (below)
+  /// due it being removed as an operand of one of its uses.  Ensure that misuse
+  /// of leaf nodes as inner nodes cannot occur by remembering all of the future
+  /// leaves and refusing to reuse any of them as inner nodes.
+  SmallPtrSet<Value*, 8> NotRewritable;
+  for (unsigned i = 0, e = Ops.size(); i != e; ++i)
+    NotRewritable.insert(Ops[i].Op);
+
   // ExpressionChanged - Non-null if the rewritten expression differs from the
   // original in some non-trivial way, requiring the clearing of optional flags.
   // Flags are cleared from the operator in ExpressionChanged up to I inclusive.
@@ -653,12 +667,14 @@
       // the old operands with the new ones.
       DEBUG(dbgs() << "RA: " << *Op << '\n');
       if (NewLHS != OldLHS) {
-        if (BinaryOperator *BO = isReassociableOp(OldLHS, Opcode))
+        BinaryOperator *BO = isReassociableOp(OldLHS, Opcode);
+        if (BO && !NotRewritable.count(BO))
           NodesToRewrite.push_back(BO);
         Op->setOperand(0, NewLHS);
       }
       if (NewRHS != OldRHS) {
-        if (BinaryOperator *BO = isReassociableOp(OldRHS, Opcode))
+        BinaryOperator *BO = isReassociableOp(OldRHS, Opcode);
+        if (BO && !NotRewritable.count(BO))
           NodesToRewrite.push_back(BO);
         Op->setOperand(1, NewRHS);
       }
@@ -682,7 +698,8 @@
         Op->swapOperands();
       } else {
         // Overwrite with the new right-hand side.
-        if (BinaryOperator *BO = isReassociableOp(Op->getOperand(1), Opcode))
+        BinaryOperator *BO = isReassociableOp(Op->getOperand(1), Opcode);
+        if (BO && !NotRewritable.count(BO))
           NodesToRewrite.push_back(BO);
         Op->setOperand(1, NewRHS);
         ExpressionChanged = Op;
@@ -695,7 +712,8 @@
     // Now deal with the left-hand side.  If this is already an operation node
     // from the original expression then just rewrite the rest of the expression
     // into it.
-    if (BinaryOperator *BO = isReassociableOp(Op->getOperand(0), Opcode)) {
+    BinaryOperator *BO = isReassociableOp(Op->getOperand(0), Opcode);
+    if (BO && !NotRewritable.count(BO)) {
       Op = BO;
       continue;
     }

Modified: llvm/branches/release_32/test/Transforms/Reassociate/crash.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/release_32/test/Transforms/Reassociate/crash.ll?rev=168489&r1=168488&r2=168489&view=diff
==============================================================================
--- llvm/branches/release_32/test/Transforms/Reassociate/crash.ll (original)
+++ llvm/branches/release_32/test/Transforms/Reassociate/crash.ll Thu Nov 22 00:21:31 2012
@@ -153,3 +153,22 @@
   %ret = add i32 %tmp2, %tmp3
   ret i32 %ret
 }
+
+; PR14060
+define i8 @hang(i8 %p, i8 %p0, i8 %p1, i8 %p2, i8 %p3, i8 %p4, i8 %p5, i8 %p6, i8 %p7, i8 %p8, i8 %p9) {
+  %tmp = zext i1 false to i8
+  %tmp16 = or i8 %tmp, 1
+  %tmp22 = or i8 %p7, %p0
+  %tmp23 = or i8 %tmp16, %tmp22
+  %tmp28 = or i8 %p9, %p1
+  %tmp31 = or i8 %tmp23, %p2
+  %tmp32 = or i8 %tmp31, %tmp28
+  %tmp38 = or i8 %p8, %p3
+  %tmp39 = or i8 %tmp16, %tmp38
+  %tmp43 = or i8 %tmp39, %p4
+  %tmp44 = or i8 %tmp43, 1
+  %tmp47 = or i8 %tmp32, %p5
+  %tmp50 = or i8 %tmp47, %p6
+  %tmp51 = or i8 %tmp44, %tmp50
+  ret i8 %tmp51
+}





More information about the llvm-branch-commits mailing list