[PATCH] D19984: [LV] Preserve order of dependences in interleaved accesses analysis

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 8 15:15:49 PDT 2016


anemet added inline comments.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5254-5258
@@ -5162,1 +5253,7 @@
 
+      // If A and B cannot be reordered, we can't add additional instructions
+      // to A's group since this may cause illegal code motion. If B is already
+      // in a group release it.
+      if (!canReorderMemAccesses(&*II, &*I)) {
+        if (isInterleaved(B)) {
+          InterleaveGroup *StoreGroup = getInterleaveGroup(B);
----------------
mssimpso wrote:
> anemet wrote:
> > Matt, I am not sure I follow, which of these accesses is not constant distance in my example?
> > 
> > I just tried this example:
> > 
> > 
> > ```
> > void
> > f(char *a, char * __restrict c, int n) {
> >   for (int i = 0; i < n; i+=3) {
> >     c[i] = a[i];
> >     a[i+4] = c[i+4];
> >     c[i+1] = a[i+1];
> >   }
> > }
> > 
> > ```
> > If you compile this on x86_64 with:
> > 
> > ```
> > -O3 -mllvm -enable-load-pre=0 -mllvm -store-to-load-forwarding-conflict-detection=0 -mllvm -enable-interleaved-mem-accesses -mllvm -force-vector-width=2
> > ```
> > 
> > Then the loads from 'a' will be merged which breaks the forward dependence between  the store of a[i+4] and the load of a[i+1].
> > 
> > No?
> Adam,
> 
> We may be talking past each other a bit. I think your original request (correct me if I'm wrong) was for a test case showing that the existing analysis doesn't consider checking memory accesses that are not "candidates" for interleaved groups. Candidate accesses are the ones collected in StrideAccesses prior to the analysis. However, every access in your example actually is a candidate access because they are all strided. They are not ignored. Yes, we currently do the wrong thing here, but that's because the current analysis isn't correctly checking dependences between the actual candidate accesses, not because it isn't considering a dependence between a candidate access and a non-candidate access.
> 
> Are you asking for something different?
Ah, the disconnect was that I didn't realize that accesses with offsets larger than the stride were considered as candidates.  The idea was to have these mimic non-strided accesses.

I guess it makes sense to consider these candidates as well because the offset is not really an offset but rather the distance between two accesses.

So I guess we can't have a testcase for this case at the moment...

I'll continue reviewing the patch.


http://reviews.llvm.org/D19984





More information about the llvm-commits mailing list