[PATCH] D16382: Add LoopSimplifyCFG pass

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

I have a couple of comments inline, after which this looks good to me.


================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:70
@@ +69,3 @@
+INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
----------------
Why do you need to depend on `TargetTransformInfoWrapperPass` and `AssumptionCacheTracker`?

================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:97
@@ +96,3 @@
+
+  for (auto Block : Blocks) {
+    // Attempt to merge blocks in the trivial case. Don't modify blocks which
----------------
I think you're copying `ValueHandle` s here (which will call `ValueHandleBase::AddToExistingUseList`).  Can you change this to `const auto &` or `auto &` ?

================
Comment at: lib/Transforms/Scalar/LoopSimplifyCFG.cpp:100
@@ +99,3 @@
+    // belong to other loops.
+    BasicBlock *Succ = dyn_cast_or_null<BasicBlock>(Block);
+    if (!Succ)
----------------
You should be able to use `cast_or_null` here.


Repository:
  rL LLVM

http://reviews.llvm.org/D16382