[PATCH] D62607: LoopDistribute/LAA: Respect convergent

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur accepted this revision.
Meinersbur added a comment.
This revision is now accepted and ready to land.

LGTM. Thank you.



================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1930
 
-  if (IsAnnotatedParallel) {
+  if (IsAnnotatedParallel/* && !HasConvergentOp*/) {
     LLVM_DEBUG(
----------------
Leftover code? Please completely remove commented-out code.


================
Comment at: lib/Transforms/Scalar/LoopDistribute.cpp:694
+    if (!LAI->canInsertRuntimeCheck() &&
+        LAI->getNumRuntimePointerChecks() != 0) {
+      return fail("RuntimeCheckWithConvergent",
----------------
arsenm wrote:
> arsenm wrote:
> > Meinersbur wrote:
> > > [serious] Isn't `PredicatedScalarEvolution` another source of runtime checks? We should ensure that it's predicate is always true. That is, LoopDistribute's condition is `!Pred.isAlwaysTrue() || !Checks.empty()`.
> > I haven't heard of PredicatedScalarEvolution. It seems if I set loop-distribute-scev-check-threshold=0, the only test that fails is symbolic-stride.ll, in the run line where  -enable-mem-access-versioning=1. It seems like there are missing tests here?
> I've spent a while trying to break this with no success. Any ideas on how to make SCEV need a check that wouldn't also have a corresponding memory dependency?
`test/Analysis/ScalarEvolution/predicated-trip-count.ll` has a case.


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

https://reviews.llvm.org/D62607