[PATCH] Vectorizing Global Structures - Take 2

[Arnold Schwaighofer]

On Feb 12, 2013, at 12:25 PM, Renato Golin <renato.golin at linaro.org> wrote:

> Hi Arnold,
> 
> Thanks for the review!
> 
> 
> On 12 February 2013 17:43, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> The existing Analysis tests for "overlap" between memory regions (underlying pointers). As such, it can ignore the access pattern and can operate using just the pointer and the underlying object.
> 
> Hum, ok, that should simplify a lot the code, then. Though, I could only find the getLocation() with StoreInst, LoadInst, VAArgInst, not generic values.

I was referring to the existing analysis in the loop vectorizer. If we start using alias analysis we have to be a lot more careful, we have to compare the pointer operand (plus consider that the access will be vectorized).

> 
> We will have to ask "Do the future vectorized accesses overlap?" Something like:
> 
> AA->alias((&A[i+1], MaxVF*4), (&A[i], MaxVF*4)).
> 
> I know, there was a comment on the previous patch to that effect. I couldn't find an AA method to inspect memory considering strides. It might be possible to examine the edges, but it'd be more efficient to do that inside AA. I assume this is done elsewhere, maybe I could re-use the same idea.

Alias analysis is not dependence analysis and as such it does not know about strides. It will only answer questions such as "Does access to location A and access to location B possibly alias". The Location object encapsulates how big our access is and the address we are accessing.


> 
> 
> Also, for the writes you use the underlying object to look up a value in the ReadWrites multi-map that is populated with "getPointerOperand()" which is not necessarily the underlying object.
> 
> That is how the vector was populated, I just stored the instruction alongside it to retrieve later on. From what you said above, it might not be necessary (but I'd have to have a way to check aliasing on the underlying objects).
I think you misunderstood my first sentence in the previous email (see above).

Think of the underlying object in A[i] as A. When you do a store or load the "getPointerOperand()" in my example is &A[i]. Two different objects - one is the gep the other one could be for example a global variable.

The existing analysis in the loop vectorizer is conservative and will say if there is both a load and store to A via two different pointers (for example &A[i], &A[i+1] it will give up.

If we add a query using alias analysis (we now allow several accesses of a common underlying object) we need to look at all accesses &X[…] for possible aliasing with &A[i] where X == A. (This is assuming we insert dynamic checks for unknown objects). 

Note the difference between &A[i] , which is what Store->getPointerOperand() returns, and A which is what GetUnderlyingObject(Store->getPointerOperand()) returns.

If we see a store to &A[i] we now need to look at all other memory accesses to see whether they alias with it. In the multimap you only store &A[i] so you can't query for all  other objects accessing A.

At a high high level (I am not suggesting we do this because it is slow), you would have to ask for every load/store pair whether their pointer operands alias under the assumption that they are accessed with a location.size that is the size of the vectorized load/store access. Everything beyond that is optimization - that we have to ensure is correct.

I also believe that if you implement this, that alias analysis will tell you that you have possibly overlapping accesses in your example:

struct {
int a[100];
int b[100];
} S;

because

alias((&S.a[99], 4xsizeof(int)), &S.b[0], 4xsizeof(int)) == partial alias


> I spent some time looking at that part of the code and I'm still in doubt as to what that possible regions can the underlying object point to, or if the underlying object is always accessible via the pointer operand.

The underlying object can point to anything - we are looking only at identified objects (Allocas, Globals, NoAlias parameters, etc.) in the existing code.
> 
> The code seems marginally ok, since all the other vectorization cases are still working and the new global alias also works, but that doesn't say much...
> 

Unfortunately, the code as it is incorrect.

> cheers,
> --renato


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130212/16c98d5a/attachment.html>