[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 4 inline comments as done.
Whitney added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:61
 class PHINode;
+class PostDominatorTree;
+class ScalarEvolution;
----------------
fhahn wrote:
> Unused?
right, will remove.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:636
+    /// Get the direction of the loop.
+    Direction getDirection() const;
+
----------------
fhahn wrote:
> This is basically the same as InductionDescriptor::getConsecutiveDirection
I believe getConsecutiveDirection only work for constant step.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:657
+    // The final value of the loop induction variable
+    Value &FinalIVValue;
+
----------------
fhahn wrote:
> I am not sure what this means exactly. Do you have checks to exclude loops with multiple exit blocks?
> 
>  Also, I am not sure what you intend on using this value for. Wouldn't it be less fragile to use SCEV to reason about the number of iterations? You can also evaluate the AddRec expression of the induction PHI at the final loop iteration.
This is the focus on the loop latch as the exiting block.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:319
+
+PHINode *Loop::getInductionVariable(ScalarEvolution &SE) const {
+  if (!isLoopSimplifyForm())
----------------
fhahn wrote:
> fhahn wrote:
> > Isn't the main part of this function the same as InductionDescriptor::isInductionPHI() ? http://llvm.org/doxygen/classllvm_1_1InductionDescriptor.html#a1974b8e8a8bae482a772ebfab1e9c794
> at leas conceptually
getInductionVariable() should return an induction phi, but an induction variable may not be a loop induction variable. A loop induction variable is used direct/indirectly in loop latch to determine if the loop continues. 
I will look into if I can further simplify this function.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60565