[PATCH] D20665: Claim NoAlias if two GEPs index different fields of the same struct

[Daniel Berlin via llvm-commits]

dberlin added a subscriber: dberlin.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:841
@@ +840,3 @@
+  // If both GEP1 and GEP2 have the inbounds keyword but index different fields
+  // of the same struct, they do not alias
+  if (GEP1->isInBounds() && GEP2->isInBounds()) {
----------------
mcrosier wrote:
> Please add a period.
This loop never verifies they are the same struct, just A struct.
They have to be the same struct, otherwise it is entirely possible for them to be different structs, and be at (offset,size) that conflict.

In general, your entire calculation is essentially trying to figure out (offset, size)  of the entire set of accesses.

It happens that, for the same struct, you can guarantee that if they go to different (offset, size) they can't alias (because you can't really overlay the structs in a different way).

But this is not, in general, true because you don't know where they are really starting from unless you see the allocation site (IE this could be a gep of a gep of a gep of a ...)

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:867
@@ +866,3 @@
+      if (isa<StructType>(Ty))
+        return NoAlias;
+      break;
----------------
This is not right. Just because what is being loaded is a struct type, means nothing.
As above, it's possible to overlay the structs in memory.

(Because LLVM is not C, things that are not legal in C *are* legal in LLVM).

You need to verify that what they are *accessing through* is the same struct type, as the top of your loop says.

Not that the loaded element is a struct type.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:868
@@ +867,3 @@
+        return NoAlias;
+      break;
+    }
----------------
This loop is really hard to follow the logic of because of where it breaks and continues.



http://reviews.llvm.org/D20665