[PATCH] Handle accesses to sturctures with embedded arrays and different offsets better

[Nick Lewycky]

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:979
@@ -978,1 +978,3 @@
 
+/// isActualObject -  Return true if V represents an actual object where we can
+/// reason about the offsets directly, like an argument, alloca, or global
----------------
I really don't understand this comment, either from "actual" or "reason about the offsets directly".

Looking at the uses, I don't see any reason not to use llvm::isIdentifiedObject. I think what you need to know is that [V, V+Offset] does not overlap any other object. isIdentifiedObject should be sufficient for that.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:983
@@ +982,3 @@
+static bool isActualObject(const Value *V) {
+  if (isa<GlobalValue>(V) || isa<Argument>(V) || isa<AllocaInst>(V))
+    return true;
----------------
What about GlobalAlias? GlobalAlias isa GlobalValue.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1063
@@ -1054,1 +1062,3 @@
+        // If they have the same offsets, and the underlying object is noalias,
+        // they must not alias
         if (GEP1BaseOffset == GEP2BaseOffset &&
----------------
Full stop after "alias".

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1069
@@ +1068,3 @@
+      } else {
+        // If these are both  arguments, global variable, or alloca, and the
+        // [offset, offset+size] ranges don't overlap, they can't alias
----------------
Extra space between "both" and "arguments".

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1069
@@ +1068,3 @@
+      } else {
+        // If these are both  arguments, global variable, or alloca, and the
+        // [offset, offset+size] ranges don't overlap, they can't alias
----------------
nlewycky wrote:
> Extra space between "both" and "arguments".
Tense agreement between "arguments" and "global variable" and "alloca".

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1070
@@ +1069,3 @@
+        // If these are both  arguments, global variable, or alloca, and the
+        // [offset, offset+size] ranges don't overlap, they can't alias
+        if (isActualObject(UnderlyingV1) && isActualObject(UnderlyingV2) &&
----------------
Full stop after "alias".

================
Comment at: test/Analysis/BasicAA/gep-alias.ll:235
@@ +234,3 @@
+
+; We should be able to determine the second store and the load do not conflict, 
+; but the first store and the load are must alias
----------------
But they can, I just need to call @test13 with %a = %b + 13 . Did you mean to check whether arguments are noalias?

%6 is %b+17 (aka gep %5, i32 0, i32 1, i32 2)
%10 loads %a+5 (aka gep %a, i32 0, i32 1, i32 0, i32 2)

[I assume one byte of padding in %struct.S00 after the i16.]

================
Comment at: test/Analysis/BasicAA/gep-alias.ll:236
@@ +235,3 @@
+; We should be able to determine the second store and the load do not conflict, 
+; but the first store and the load are must alias
+define i32 @test13(%struct.S03* %a, %struct.S03* %b) {
----------------
Full stop after "alias".

http://reviews.llvm.org/D8487

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/