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

[Luofan Chen via Phabricator via llvm-commits]

bbn 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 =
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86696