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

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis 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:
> uenoku wrote:
> > baziotis wrote:
> > > uenoku wrote:
> > > > 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)
> > > 1) Well, it gets really complicated (and hacky) really fast. You have to have two code paths; one for rotated and one for not. Also, I agree with Kuter in the other comment. For an "always true" condition, the range has to be _equal_ to 1, not just contain it.
> > > 
> > > But even with an always true condition, you're not sure the loop is endless.
> > > Because the loop might have some kind of control
> > > flow inside that makes the loop stop. For example:
> > > ```
> > > while (some condition that is always true) {
> > >   if (some condition that makes the loop stop at some point)
> > >     break;
> > > }
> > > ```
> > > Which brings us again to the initial point: It seems using SCEV is the only sane way. SCEV is
> > > supposed to handle all that and other analyses / transformations to use it, instead of replicating
> > > part of its functionality.
> > > 
> > > 2) 
> > > > I think SCEV could be a good idea, but I cannot find a reasonable test for this
> > > 
> > > For what ?
> > > 
> > > > The range I get is always "full-set", can you give an example for this?
> > > 
> > > @uenoku should be able to help you here.
> > > I think even if the loop is rotated, we can use such method to determine whether a loop is endless or not, right
> > Maybe we can first check the loop is canonicalized or not. If the loop is LCSSA, things get easier. 
> > But as @baziotis says we need to replicate the code so it's not clear we should do similar things here.
> > One solution would be adding callbacks to SCEV and using Attributor information from SCEV but it would be (really) complicated and difficult. 
> Thanks for the idea here. I will take a look at it.
> Maybe we can first check the loop is canonicalized or not. If the loop is LCSSA, things get easier.

How would LCSSA help ? Btw, @bbn if you want to read about LCSSA or other canonical forms, you can check this: https://llvm.org/docs/LoopTerminology.html

> One solution would be adding callbacks to SCEV and using Attributor information from SCEV but it would be (really) complicated and difficult.

I mean yes, ideally SCEV would do its thing by taking advantage of the Attributor. But right now, it's certainly a bad idea unless we have a good plan for it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86696