[PATCH] Make ScalarEvolution::ComputeExitLimit more agressive

Ramkumar Ramachandra artagnon at gmail.com
Sun Jan 11 15:02:59 PST 2015


Cursory review.


================
Comment at: include/llvm/IR/BasicBlock.h:214
@@ +213,3 @@
+  ///
+  /// Note that unique successir doesn't mean single edge, there can be
+  /// multiple edges from this block to the unique successor (for example a
----------------
s/successir/successor/

================
Comment at: include/llvm/IR/BasicBlock.h:217
@@ +216,3 @@
+  /// switch statement with multiple cases having the same destination).
+  BasicBlock *getUniqueSuccessor();
+  const BasicBlock *getUniqueSuccessor() const {
----------------
Slightly tangential: is a hypothetical getSingleSucessor() not useful yet?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:4836
@@ +4835,3 @@
+    while (true) {
+      if (std::next(pred_begin(UniqueSucc)) != pred_end(UniqueSucc))
+        return false;
----------------
Um, pred_size()?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:4840
@@ +4839,3 @@
+      BasicBlock *Next = UniqueSucc->getUniqueSuccessor();
+      if (!Next || Next == L->getHeader())
+        break;
----------------
Are you worried that `L->getHeader()` might be `nullptr`? Why check `Next` separately then?

================
Comment at: lib/Analysis/ScalarEvolution.cpp:4866
@@ -4835,3 +4865,3 @@
       Exit = *SI;
-    } else if (*SI != L->getHeader()) {
-      MustExecuteLoopHeader = false;
+    } else if (*SI != L->getHeader() && MustExecuteLoopHeader) {
+      // TODO: DominatesHeaderIgnoringLoopEntry cannot be true for two different
----------------
Redundant `&& MustExecuteLoopHeader` -- you do an `&=` anyway.

================
Comment at: lib/IR/BasicBlock.cpp:249
@@ +248,3 @@
+  succ_iterator SI = succ_begin(this), E = succ_end(this);
+  if (SI == E) return nullptr; // No preds.
+  BasicBlock *SuccBB = *SI;
----------------
succ_empty()?

================
Comment at: lib/IR/BasicBlock.cpp:252
@@ +251,3 @@
+  ++SI;
+  for (;SI != E; ++SI) {
+    if (*SI != SuccBB)
----------------
Probably nicer with a range-based loop or std::all_of.

================
Comment at: test/Analysis/ScalarEvolution/compute-exit-limit-aggressively.ll:72
@@ +71,3 @@
+; CHECK: Determining loop execution counts for: @h
+; CHECK-NEXT: Loop %loop: Unpredictable backedge-taken count.
+; CHECK-NEXT: Loop %loop: Unpredictable max backedge-taken count.
----------------
Why have you shown two of these examples with the unknown parameter passed?

http://reviews.llvm.org/D6830

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list