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

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 11 10:33:18 PDT 2019


fhahn added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:639
+  /// each time through the loop.
+  bool isCanonical(ScalarEvolution *SE = nullptr) const;
+
----------------
Whitney wrote:
> fhahn wrote:
> > Isn't that unnecessarily strict?
> There exists getCanonicalInductionVariable() which returns the first PHINode which starts at 0 and increments by one each time through the loop. I would like to use the same terminology here.
Right. I *thought* getCanonicalInductionVariable was kind of deprecated in favor of the more general InductionDescriptor, but I think that was just mentioned as a review comment a while back.


================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:213
+/// Return true if V1 == V2 + 1
+static bool diffOne(Value &V1, Value &V2, ScalarEvolution *SE = nullptr) {
+  if (V1.getType() != V2.getType())
----------------
Whitney wrote:
> fhahn wrote:
> > I guess the manual handling might be faster in some cases, but this seems like a thing SE should handle for us. Would it be possible to just require it and use it?
> The ideal here is trying to get these loop information without the requirement of SE, but optionally takes SE for better result.
Sure, but we end up trying to do lots of things SE already does. Are you having a use case in mind where SE is not available? AFAIK, most loop passes require SE already, to do similar kinds of reasoning.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60565





More information about the llvm-commits mailing list