[PATCH] D29425: [SLP] Use SCEV to sort memory accesses

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 2 12:01:35 PST 2017

efriedma added inline comments.

Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1093-1095
+              const SCEVConstant *Diff = dyn_cast<SCEVConstant>(
+                  SE.getMinusSCEV(Right.first, Left.first));
+              return (Diff && (Diff->getAPInt().isStrictlyPositive()));
mkuper wrote:
> mssimpso wrote:
> > Do we require constant differences? If not, you could also ask SCEV about the range:
> > 
> > ```
> > return SE.isKnownPositive(SE.getMinusSCEV(Right.first, Left.first))
> > ```
> @mssimpso : right, I think this should work, thanks! I was stuck thinking about constants because if the differences are non-constant we won't be able to do anything with the sorted array anyway. But this is nicer assuming I can actually use it as a comparator (see answer to Eli).
> @efriedma : I know. That's why I've wanted sanjoy to comment on whether we can rely on SCEV to give transitive results. I mean, this comparator is obviously irreflexive, and I'm rather sure it's antisymmetric. So it comes down to whether we can rely on isKnownPositive(getMinus(A,B)) and isKnownPositive(getMinus(B,C)) implying that isKnownPositive(getMinus(A,C)) (or the equivalent for the version I have now).
> If not, then I see two options:
> (a) roll my own sort that bails if two values are incomparable.
> (b) do something like: take one of the pointers, compute the difference vs. all the others, if one of the differences is non-constant bail, otherwise compare based on the diffs.
> I guess (b) would be the simplest and safest option.
I don't think we can rely on SCEV being transitive?  At least, I wouldn't want to rely on it, then crash in some obscure case.

Anyway, your comparator has worse problems: currently, you can prove 0 == A == 1 with it.


More information about the llvm-commits mailing list