[PATCH] Allow global structures to be vectorized

[Arnold Schwaighofer]

Hi Renato,

I don't think this is correct. You are asserting that: "Two geps will only alias if they have the same elements in the index list"?

-      if (WriteObjects.count(*it)) {
-        DEBUG(dbgs() << "LV: Found a possible read/write reorder:"
-              << **it <<"\n");
-        return false;
+      // We want to know the offset of GEPs, too
+      std::pair<GEPOperator*, OffsetList> GEP = findGEPOperator(*I, Read, DL);
+      if (GEP.first)
+        Read = GEP.first;
+      if (WriteObjects.count(Read)) {
+        // Non-GEPs are directly related
+        if (!GEP.first) {
+          DEBUG(dbgs() << "LV: Found a possible read/write reorder: "
+                << *Read <<"\n");
+          return false;
+        }
+        // For GEPs Make sure they're not just different offsets
+        OffsetList& WriteList = (*WriteObjects.find(Read)).second;
+        OffsetList& ReadList = GEP.second;
+        if (WriteList == ReadList) {
+          DEBUG(dbgs() << "LV: Found a possible read/write reorder: "
+                << *Read <<"\n");
+          return false;
+        }
+        // TODO: Use vector size for further conflict detection?

This is not correct. To make any statement about aliasing of two GEPs you need to symbolically evaluate them (and their recursive inputs). Look at BasicAA::aliasGEP. 

Also note, LLVM IR's semantic does *not* allow you to treat two GEP access like an index into a type tree, and assume if the paths are different you don't have an aliasing memory.

Just because you have:

struct {
   int B[100];
   int A[100];
} S;

You cannot assume that

gep S, 0, 1, x
gep S, 0, 0, y

don't alias for any x,y. LLVM's IR semantics allows you to walk beyond B[99]. Using gep S, 0, 0, 100 is perfectly legal in llvm ir.


Similarly, you cannot assume that 

gep S, 0, x, y
gep S, 0, z, a

alias only if x ==z and y ==a.

Best,
Arnold


On Feb 6, 2013, at 10:17 AM, Renato Golin <renato.golin at linaro.org> wrote:

> Hi all,
> 
> Far from perfect, the patch adds an extra check on the offsets. I may not be using the right containers, and the static function findGEPOperator() is ill-placed, but that's the basic idea behind checking for GEPs. Any comments are really appreciated.
> 
> Another thing I'd like to ask is about the vector size.
> 
> +  // TODO: Use vector size for further conflict detection?
> 
> Should I investigate if the offsets point to a reasonable distance in memory, given the vector size?
> 
> If the write is on P and the read is on P+(2 x int) and the vector size is (4 x int), they do alias, even though the GEP is not on the same element.
> 
> I also may have to check that the address space of both pointers is the same, and possibly add this to the ValueOffsetMap container... or e may assume that address spaces will never overlap on reasonable vector sizes (ie, they're not contiguous on at least 1 vector distance).
> 
> cheers,
> --renato
> <global_vectorize.patch>