[PATCH] D20495: [BasicAA] An inbounds GEP and an alloca can't alias if the base of the GEP would point "below" the alloca

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:346
@@ -345,3 +345,3 @@
 /// through pointer casts.
-/*static*/ const Value *BasicAAResult::DecomposeGEPExpression(
-    const Value *V, int64_t &BaseOffs,
+const Value *BasicAAResult::DecomposeGEPExpression(
+    const Value *V, int64_t &BaseStructOffs, int64_t &BaseNonStructOffs,
----------------
Document parameter BaseStructOffs, and BaseNonStructOffs.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:954
@@ -951,1 +953,3 @@
 
+// If a we have (a) a GEP and (b) a pointer based on an alloca, and the
+// beginning of the object the GEP points would have a negative offset with
----------------
Probably add an example in C form to demonstrate it. Also document key parameters of the method.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:968
@@ +967,3 @@
+  // If the alloca access size is unknown, or the GEP isn't inbounds, bail.
+  if (GEPMaxLookupReached || AllocaMaxLookupReached ||
+      AllocaAccessSize == MemoryLocation::UnknownSize || !GEPOp->isInBounds())
----------------
Since this method does not need to update the two limits, it is better to move their checks out of line before the call. i.e.:

   if (!GEPMaxLookupReached && !AllocaMaxLookupReached && isGEPBaseAtNegativeOffset(..))
       return NoAlias;

This will help make the interface cleaner.

The limit check can also be combined with the next call:

  if (!GEPMaxLookupReached && !AllocaMa... ) {
       if (!isGEP....) 
          return NoAlias;
        if (!isGEP...)
           return NoAlias;
     }


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1035
@@ +1034,3 @@
+    if (isGEPBaseAtNegativeOffset(GEP2StructOffset, GEP2NonStructOffset,
+                                  GEP1BaseOffset, !GEP2VariableIndices.empty(),
+                                  !GEP1VariableIndices.empty(),
----------------
Perhaps passing the GEPVariableIndices directly to the callee.


http://reviews.llvm.org/D20495