From 2d5f8ca3d180832d168e59e2bf3d85317e86287d Mon Sep 17 00:00:00 2001
From: Duncan Sands <baldrick@free.fr>
Date: Wed, 27 Jun 2012 14:19:00 +0000
Subject: Some reassociate optimizations create new instructions, which they
 insert just before the expression root.  Any existing operators that are
 changed to use one of them needs to be moved between it and the expression
 root, and recursively for the operators using that one.  When I rewrote
 RewriteExprTree I accidentally inverted the logic, resulting in the
 compacting going down from operators to operands rather than up from operands
 to the operators using them, oops.  Fix this, resolving PR12963.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@159265 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Transforms/Scalar/Reassociate.cpp | 18 +++++++-----------
 test/Transforms/Reassociate/crash.ll  | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lib/Transforms/Scalar/Reassociate.cpp b/lib/Transforms/Scalar/Reassociate.cpp
index b572f4eb07..69cdc0f799 100644
--- a/lib/Transforms/Scalar/Reassociate.cpp
+++ b/lib/Transforms/Scalar/Reassociate.cpp
@@ -673,16 +673,10 @@ void Reassociate::RewriteExprTree(BinaryOperator *I,
   // original in some non-trivial way, requiring the clearing of optional flags.
   // Flags are cleared from the operator in ExpressionChanged up to I inclusive.
   BinaryOperator *ExpressionChanged = 0;
-  BinaryOperator *Previous;
-  BinaryOperator *Op = 0;
-  for (unsigned i = 0, e = Ops.size(); i != e; ++i) {
+  for (unsigned i = 0; ; ++i) {
     assert(!NodesToRewrite.empty() &&
            "Optimized expressions has more nodes than original!");
-    Previous = Op; Op = NodesToRewrite.pop_back_val();
-    if (ExpressionChanged)
-      // Compactify the tree instructions together with each other to guarantee
-      // that the expression tree is dominated by all of Ops.
-      Op->moveBefore(Previous);
+    BinaryOperator *Op = NodesToRewrite.pop_back_val();
 
     // The last operation (which comes earliest in the IR) is special as both
     // operands will come from Ops, rather than just one with the other being
@@ -771,15 +765,17 @@ void Reassociate::RewriteExprTree(BinaryOperator *I,
   }
 
   // If the expression changed non-trivially then clear out all subclass data
-  // starting from the operator specified in ExpressionChanged.
-  if (ExpressionChanged) {
+  // starting from the operator specified in ExpressionChanged, and compactify
+  // the operators to just before the expression root to guarantee that the
+  // expression tree is dominated by all of Ops.
+  if (ExpressionChanged)
     do {
       ExpressionChanged->clearSubclassOptionalData();
       if (ExpressionChanged == I)
         break;
+      ExpressionChanged->moveBefore(I);
       ExpressionChanged = cast<BinaryOperator>(*ExpressionChanged->use_begin());
     } while (1);
-  }
 
   // Throw away any left over nodes from the original expression.
   for (unsigned i = 0, e = NodesToRewrite.size(); i != e; ++i)
diff --git a/test/Transforms/Reassociate/crash.ll b/test/Transforms/Reassociate/crash.ll
index e2ebcddf6b..a99c844f01 100644
--- a/test/Transforms/Reassociate/crash.ll
+++ b/test/Transforms/Reassociate/crash.ll
@@ -119,3 +119,17 @@ for.cond:                                         ; preds = %for.cond, %entry
   %conv = zext i16 %p to i32
   br label %for.cond
 }
+
+; PR12963
+@a = external global i8
+define i8 @f0(i8 %x) {
+  %t0 = load i8* @a
+  %t1 = mul i8 %x, %x
+  %t2 = mul i8 %t1, %t1
+  %t3 = mul i8 %t2, %t2
+  %t4 = mul i8 %t3, %x
+  %t5 = mul i8 %t4, %t4
+  %t6 = mul i8 %t5, %x
+  %t7 = mul i8 %t6, %t0
+  ret i8 %t7
+}
-- 
cgit v1.2.3