[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