[PATCH] D21000: [CFLAA] Cleaned up StratifiedAttrs handling

[Jia Chen via llvm-commits]

grievejia added inline comments.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1099-1117
@@ -1089,21 +1098,21 @@
 
-  // Stratified set attributes are used as markets to signify whether a member
-  // of a StratifiedSet (or a member of a set above the current set) has
-  // interacted with either arguments or globals. "Interacted with" meaning its
-  // value may be different depending on the value of an argument or global. The
-  // thought behind this is that, because arguments and globals may alias each
-  // other, if AttrsA and AttrsB have touched args/globals, we must
-  // conservatively say that they alias. However, if at least one of the sets
-  // has no values that could legally be altered by changing the value of an
-  // argument or global, then we don't have to be as conservative.
-  if (AttrsA.any() && AttrsB.any())
+  // If both values are local (meaning the corresponding set has attribute
+  // AttrNone or AttrEscaped), then we know that CFLAA fully models them: they
+  // may-alias each other if and only if they are in the same set
+  // If at least one value is non-local (meaning it either is global/argument or
+  // it comes from unknown sources like integer cast), the situation becomes a
+  // bit more interesting. We follow three general rules described below:
+  // - Non-local values may alias each other
+  // - AttrNone values do not alias any non-local values
+  // - AttrEscaped values do not alias globals/arguments, but they may alias
+  // AttrUnknown values
+  if (SetA.Index == SetB.Index)
     return MayAlias;
-
-  // We currently unify things even if the accesses to them may not be in
-  // bounds, so we can't return partial alias here because we don't know whether
-  // the pointer is really within the object or not.
-  // e.g. Given an out of bounds GEP and an alloca'd pointer, we may unify the
-  // two. We can't return partial alias for this case. Since we do not currently
-  // track enough information to differentiate.
-  return SetA.Index == SetB.Index ? MayAlias : NoAlias;
+  if (AttrsA.none() || AttrsB.none())
+    return NoAlias;
+  if (AttrsA.test(AttrUnknownIndex) || AttrsB.test(AttrUnknownIndex))
+    return MayAlias;
+  if (isGlobalOrArgAttr(AttrsA) && isGlobalOrArgAttr(AttrsB))
+    return MayAlias;
+  return NoAlias;
 }
----------------
george.burgess.iv wrote:
> grievejia wrote:
> > Can you explain your suspicion that "we'll need to make it (3 x 4) MayAlias in the near future"? I thought the definition of AttrEscaped has already eliminate this possibility, since (3) only includes locally identifiable memory objects (allocas and mallocs) and those objects shouldn't alias any global/argument.
> It's my understanding that our plan is to eventually mark call args as AttrEscaped instead of AttrUnknown. With everything in its current state, it seems that doing so would break in a case like:
> 
> ```
> int *G;
> // assume this is external to CFLAA
> void f(uintptr_t A) {
>   G = (int *)A;
> }
> 
> void callF() {
>   int S;
>   int *P = &S;
>   f((uintptr)P);
>   int *PAlias = G;
> }
> ```
> 
> If we're not doing this, then (3 x 4) can stay NoAlias. :)
I see the problem here. Thanks for the example. It's very instructive. 


Repository:
  rL LLVM

http://reviews.llvm.org/D21000