[PATCH] D29488: [DA] Fix for PR31848: Treat AddRec subscripts containing extra loops as NonLinear

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello. This looks familiar :)

There is a testcase in https://reviews.llvm.org/D46197 (which this is probably a better fix for), with a store outside any loop, but with this same case of the location being an addrec from the preceeding loop. It's not a big problem in that case as the store is not in a loop, making it mostly uninteresting to DA. But the -analyze option will try to look at it, triggering an assert. I think it's not currently handled by this patch because of the if (LCA && ...) check.

Worth fixing here or should we just ignore that case? Perhaps just not analyse loads/stores outside of loops in -analyze?



================
Comment at: lib/Analysis/DependenceAnalysis.cpp:871
   const SCEV *Step = AddRec->getStepRecurrence(*SE);
   const SCEV *UB = SE->getBackedgeTakenCount(AddRec->getLoop());
   if (!isa<SCEVCouldNotCompute>(UB)) {
----------------
Nit: Loop instead of AddRec->getLoop(), same as Dst


================
Comment at: lib/Analysis/DependenceAnalysis.cpp:887
     return false;
   Loops.set(mapSrcLoop(AddRec->getLoop()));
   return checkSrcSubscript(Start, LoopNest, Loops);
----------------
Same again


https://reviews.llvm.org/D29488