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

[Jia Chen via llvm-commits]

grievejia added inline comments.

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1100
@@ -1098,2 +1099,3 @@
   // argument or global, then we don't have to be as conservative.
-  if (AttrsA.any() && AttrsB.any())
+  if ((AttrsA.none() && AttrsB.any()) || (AttrsA.any() && AttrsB.none()))
+    return NoAlias;
----------------
george.burgess.iv wrote:
> This looks a bit subtle. I think we can make it less so if we move the index number check above this. If we do that, it seems that we can simplify this entire if/else chain to something like
> 
> ```
> if (a.none() || b.none() || (a == Escaped && b == Escaped))
>   return NoAlias;
> return MayAlias;
> ```
> 
> Right?
If both a.none() and b.none() are true, we still need to check SetA.Index == SetB.Index, right?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1105
@@ +1104,3 @@
+  else if ((AttrsA & StratifiedAttrs(AttrEscaped)).any() &&
+           (AttrsB & StratifiedAttrs(AttrEscaped)).any())
+    return NoAlias;
----------------
george.burgess.iv wrote:
> `AttrsA.test(AttrEscapedIndex) && AttrsB.test(AttrEscapedIndex)`?
> 
> Either way, this looks like it'll hand us NoAlias if I have e.g. two sets marked with every StratifiedAttr except AttrUnknown.
If we go with Attrs.test(AttrEscapedIndex), the problem is that we may miss the check for global/argument attrs. They are essentially the same as AttrUnknown except that we are aware why they are unknown. 

Maybe I'll restructure the checks a little bit here and try to merge the check of global/argument/AttrUnknown together, to make it more clear. 


http://reviews.llvm.org/D21000