[PATCH] D110973: [DA] Handle mismatching loop levels by considering them non-linear

[Congzhe Cao via Phabricator via llvm-commits]

congzhe added inline comments.


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:917
+  DispositionTy Disposition = SE->getLoopDisposition(AddRec, LoopNest);
+  if (Disposition != DispositionTy::LoopInvariant &&
+      Disposition != DispositionTy::LoopComputable)
----------------
bmahjour wrote:
> congzhe wrote:
> > congzhe wrote:
> > > Thanks for the change, It looks good to me. I don't know if Micheal has further comments?
> > > 
> > > [nit] I'm wondering if it would be better to simplify the code like this?
> > > ```
> > > if (SE->getLoopDisposition(AddRec, LoopNest) == ScalarEvolution::LoopDisposition::LoopVariant )
> > >   return false;
> > > ```
> > > Thanks for the change, It looks good to me. I don't know if Micheal has further comments?
> > > 
> > > [nit] I'm wondering if it would be better to simplify the code like this?
> > > ```
> > > if (SE->getLoopDisposition(AddRec, LoopNest) == ScalarEvolution::LoopDisposition::LoopVariant )
> > >   return false;
> > > ```
> > 
> > Actually could be futher simplified to 
> > ```
> > if (SE->getLoopDisposition(AddRec, LoopNest) == ScalarEvolution::LoopVariant )
> >   return false;
> > ```
> > 
> I thought about that too, but decided to check for `LoopComputable` and `LoopInvariant` specifically, in case new enums are added to `LoopDisposition` in the future. The logic also makes a bit more sense when I think about it as neither invariant nor computable. LoopVariant isn't the best wording for what it really means.
Sure, makes sense to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110973