[PATCH] D22377: [SCEV] trip count calculation for loops with unknown stride

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 1 19:13:25 PDT 2016


sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8734
@@ +8733,3 @@
+    // effects. The following checks are used to check the same. Here's an 
+    // example of the loop we are trying to handle here-
+    // 
----------------
nit: "here -"

================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8748
@@ +8747,3 @@
+    //
+    if (!NoWrap || isKnownNonPositive(Stride) || !loopHasNoSideEffects(L))
+      return getCouldNotCompute();
----------------
I'm not super-comfortable with this -- (IIUC) you're assuming that given an unknown stride, there are certain things SCEV cannot prove.  This will create a tricky coupling between different parts and e.g. introduces the possibility that we'll get miscompiles if we make an unrelated part of SCEV smarter in some creative way (like you're doing here :) ).

I think it is better to assume that the other bits in SCEV are omniscient (i.e. they may (but are not guaranteed to) prove anything that is factually correct at runtime), and base our inferences on that.

The other thing that's missing here is some clear justification of why what you're doing is correct.  This does not necessarily have to be a formal proof (though that'd make me really happy :) ) but at least a clear statement of preconditions you've assumed, and why that implies that the trip count of `(max(rhs, start) - start + stride - 1) / u stride` is correct.


https://reviews.llvm.org/D22377





More information about the llvm-commits mailing list