[PATCH] D63885: [LOOPINFO] Introduce the loop guard API.

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 22 09:44:39 PDT 2019


reames accepted this revision.
reames added a comment.
This revision is now accepted and ready to land.

LGTM w/required changes before submit.

p.s. Your next step after this lands needs to be finding a consumer.  I'm willing to approve this speculatively, but if it sits in tree unused for long, it will be deleted.



================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:734
+  /// For rotated loops, the guard branch determines if the loop executes at
+  /// least one iteration. That is, a rotated loop with a guard branch will
+  /// execute0 or more times, depending on the evaluation of the guard. On the
----------------
Before submit, remove the text about number of times the loop executes.  This is confusing as you don't define which portion of the loop you mean, and the terminology needs word smithed.  This can be added in a follow on patch.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:363
+BranchInst *Loop::getLoopGuardBranch() const {
+  assert(isLoopSimplifyForm() && "Only valid for loop in simplify form");
+  BasicBlock *Preheader = getLoopPreheader();
----------------
In a follow on patch, these restrictions should be converted to early returns.  (i.e. there's no need to have this be a strict precondition)


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:386
+  BranchInst *GuardBI = dyn_cast_or_null<BranchInst>(GuardBB->getTerminator());
+  if (!GuardBI || GuardBI->getNumSuccessors() != 2)
+    return nullptr;
----------------
Branches only have 2 successors, so remove that check.
And replace the dyn_cast_or_null with a dyn_cast.  getTerminator can never return null on properly constructed IR.  


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63885/new/

https://reviews.llvm.org/D63885





More information about the llvm-commits mailing list