[PATCH] D60565: [LOOPINFO] Extend Loop object to add utilities to get the loop bounds, step, induction variable, and guard branch.

[Whitney via Phabricator via llvm-commits]

Whitney marked 3 inline comments as done.
Whitney added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:575
+  /// isAuxiliaryInductionVariable(x) --> true if x == i_1
+  /// isCanonical                   --> false
+  struct LoopBounds {
----------------
kbarton wrote:
> This is minor, but have you tried running this through doxygen to see what the output looks like?
> It may come out looking much different in the doxygen pages then it does here. 
Tried this with doxygen, looks as expected.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:249
+
+  BranchInst *BI = dyn_cast<BranchInst>(Latch->getTerminator());
+  assert(BI && BI->isConditional() && "Expecting conditional latch branch");
----------------
kbarton wrote:
> Should this be dyn_cast_or_null instead, to account for the possibility that getTerminator could returns nullptr?
Can a basic block not have a terminator? I thought it must have one.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:488
+  BasicBlock *Header = getHeader();
+  BasicBlock *Latch = getLoopLatch();
+  ICmpInst *CmpInst = getLatchCmpInst(*this);
----------------
kbarton wrote:
> It's probably reasonable here to assert that Header and Latch are not nullptr. 
This will definitely be true, because it just checked that the loop is in isLoopSimplifyForm()..I can add asserts.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60565