[PATCH] D86696: [Attributor][WIP] Introduce Loop AA

[Hideto Ueno via Phabricator via llvm-commits]

uenoku added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:7943
+        if (auto *BI = dyn_cast<BranchInst>(&I)) {
+          auto *Op = BI->getOperand(0);
+          const auto &OpAA =
----------------
bbn wrote:
> baziotis wrote:
> > uenoku wrote:
> > > baziotis wrote:
> > > > For that to make sense, you have to verify that the branch `isConditional()`. Also, note that loops
> > > > may be [[ https://llvm.org/docs/LoopTerminology.html#rotated-loops | rotated ]]. In that case, the condition is not in the header block, but in the latch.
> > > > 
> > > > Generally, loops can get quite complicated and so, as Kuter said, you probably want to use
> > > > SCEV (for example, backedge taken count etc.) I'll have to think that again because SCEV
> > > > won't benefit from Attributor, meaning, at a high-level, using constant range AA when it makes
> > > > sense seems reasonable.
> > > FWIW, AAConstantRange actually uses SCEV internally but I agree that using AAConstantRange might make things complicated as a starting point. So you can use SCEV first. 
> > Yes, but the point is that using constant range directly makes things difficult because we have to re-deduce information and loop structure that SCEV is supposed to handle (e.g. the code above is doing something similar to backedge taken count).
> Thanks for the idea.
> 1. I think even if the loop is rotated, we can use such method to determine whether a loop
>     is endless or not, right?
> 2. I think SCEV could be a good idea, but I cannot find a reasonable test for this. The range I
>     get is always "full-set", can you give an example for this? Thanks
> I think SCEV could be a good idea, but I cannot find a reasonable test for this. The range I get is always "full-set", can you give an example for this? 
We have tests for loop terminations in a test for `willreturn` (I guess)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86696