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

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1081
+    if (CurrObj != Obj0) {
+      Sorted.append(VL.begin(), VL.end());
+      return;
----------------
mssimpso wrote:
> Hi Michael,
> 
> I must have missed this the first time around, but this looks a little confusing to me. If the accesses aren't sortable, why are we adding them to Sorted? I would think the caller would need some signal that sorting was unsuccessful - either an empty Sorted or a returned bool. Am I missing something obvious?
No, you're not missing anything.

It's just that the API currently doesn't allow for the possibility of failure, so the callers blindly use the "sorted" array. This made sense for the previous version because it did not explicitly fail (it would just produce a meaningless order in cases where things weren't really sortable), and I didn't notice this when reviewing the original patch.
I was planning to fix the API up in a follow-up patch, but if you want to see it in the review, I can update it with that.


https://reviews.llvm.org/D29425