[PATCH] D20703: [SCEV] Keep SCEVExpander insert points consistent.

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: include/llvm/Analysis/ScalarEvolutionExpander.h:97
@@ +96,3 @@
+      ~SCEVInsertPointGuard() {
+        assert(SE->InsertPointGuards.back() == &Guard);
+        SE->InsertPointGuards.pop_back();
----------------
Add a description here on why this assert shouldn't fail.

================
Comment at: include/llvm/IR/IRBuilder.h:229
@@ +228,3 @@
+
+    BasicBlock::iterator GetInsertPoint() const { return Point; }
+    void SetInsertPoint(BasicBlock::iterator I) { Point = I; }
----------------
This new public interface looks like the kind of thing that will be inadvertently get misused later.  How about duplicating the functionality of `InsertPointGuard` in `SCEVInsertPointGuard` and adding the getter and setter on just that class?

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:917
@@ +916,3 @@
+  if (Builder.GetInsertPoint() == It)
+    Builder.SetInsertPoint(Builder.GetInsertBlock(), NewInsertPt);
+  for (auto *InsertPtGuard : InsertPointGuards)
----------------
Why not `Builder.SetInsertPoint(&*NewInsertPt)`?


http://reviews.llvm.org/D20703