[PATCH] D15058: [SCEVExpander] Have hoistIVInc preserve LCSSA

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi Sanjoy,

A couple of comments inline, otherwise the patch looks good to me.

Thanks,
Michael


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:936-938
@@ -935,2 +935,5 @@
 
+  if (!SE.LI.movementPreservesLCSSAForm(IncV, InsertPos))
+    return false;
+
   // Check that the chain of IV operands leading back to Phi can be hoisted.
----------------
With this change we should become more conservative in some cases; did you see any effect on the testsuite? E.g. how many tests would crash if we replace with condition with an assert?

================
Comment at: lib/IR/Instruction.cpp:65-70
@@ -64,3 +64,8 @@
 
+Function *Instruction::getFunction() { return getParent()->getParent(); }
+
+const Function *Instruction::getFunction() const {
+  return getParent()->getParent();
+}
 
 void Instruction::removeFromParent() {
----------------
This should  probably go in a separate patch.

================
Comment at: test/Transforms/IndVarSimplify/pr24804.ll:14-15
@@ +13,4 @@
+for.cond:                                         ; preds = %for.inc, %for.cond, %entry
+  %0 = phi i32 [ 0, %entry ], [ %add, %for.inc ], [ %0, %for.cond ]
+  %add = add nsw i32 %0, 1
+  %idxprom = sext i32 %add to i64
----------------
Maybe run instnamer on the test?


http://reviews.llvm.org/D15058