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

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Analysis/DependenceAnalysis.cpp:917
+  DispositionTy Disposition = SE->getLoopDisposition(AddRec, LoopNest);
+  if (Disposition != DispositionTy::LoopInvariant &&
+      Disposition != DispositionTy::LoopComputable)
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110973