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


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:609
+  /// else return nullptr.
+  std::unique_ptr<LoopBounds> getBounds(ScalarEvolution *SE = nullptr) const;
+
----------------
fhahn wrote:
> Do we need dynamic allocations here? The LoopBounds struct is small and trivially movable/copyable, so just returning it as a value would be simpler and probably faster.
The reason I wanted to return a unique_ptr is getBounds() may return nullptr if it doesn't find the induction variable, or unable to create the bound. And with unique_ptr, it doesn't need to worry about cleaning the underlining object of a pointer. What do you think?


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:584
+    /// - the LHS of the latch comparison is StepInst
+    ICmpInst::Predicate getCanonicalPredicate() const;
+    /// An enum for the direction of the loop
----------------
jdoerfert wrote:
> This definition seems to lose given that there are various corner cases, e.g., wrapping induction variables.
Are you talking about the getCanonicalPredicate()? I am trying to understand the problem. Can you please give me an example which doesn't work?
This function is called when there exists an induction variable, a corresponding compare instruction, and a corresponding step instruction. And it is returning the predicate of the compare instruction when the first successor of the latch branch is the loop header. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60565