[PATCH] D21110: [CFLAA] Add yet another StratifiedAttr

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

Tests, please. :)

Also, can you please go over how this doesn't make arg/global attributes basically redundant? Any set tagged with External attributes will have all sets below it tagged with External. So, it looks like this change would make it more difficult to handle interprocedural queries where we know the source of arguments.


================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:658
@@ -655,4 +657,3 @@
 
-/// Given a StratifiedAttrs, returns true if it marks the corresponding values
-/// as unknown, globals or arguments
-static bool isNonLocalAttr(StratifiedAttrs Attr);
+/// Given a StratifiedAttrs, returns true if says the corresponding values comes
+/// from unknown sources (such as opaque memory or integer cast)
----------------
Nit: Remove 'says ', 'comes' -> 'come'

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:662
@@ +661,3 @@
+
+/// Given a StratifiedAttrs, returns true if says the corresponding values comes
+/// from known sources (such as global/argument values) but may points to
----------------
'comes' -> 'come'

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:663
@@ +662,3 @@
+/// Given a StratifiedAttrs, returns true if says the corresponding values comes
+/// from known sources (such as global/argument values) but may points to
+/// unknown locations
----------------
'points' -> 'point'

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1014
@@ +1013,3 @@
+        if (MaybeCurAttr && Direction == Level::Below)
+          Builder.noteAttributes(OtherValue, *MaybeCurAttr);
+
----------------
ISTM we would end up with different attributes for the StratifiedSets in cases like:

```
void foo(int **a) {
  int **aalias = a;
  int *b = *a;
}
```

and

```
void foo(int **a) {
  int **aalias = a;
  int *b = *aalias;
}
```

...Since, when analyzing `*a`, MaybeCurAttr is marked as External (and Arg #0), whereas when looking at `*aalias`, we won't mark any attributes. After set finalization/etc, the former code sample will end with two sets, each with External and Arg #0 attrs, but the latter will produce two sets with only the External attr set.

Is this intentional?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1037
@@ -1018,5 +1036,3 @@
 
-    auto Attr = valueToAttr(&Arg);
-    if (Attr.hasValue())
-      Builder.noteAttributes(&Arg, *Attr);
+    Builder.noteAttributes(&Arg, AttrExternal);
   }
----------------
Is there a reason that we don't honor the `noalias` attribute anymore here?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1119
@@ -1102,3 +1118,3 @@
   // 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:
+  // it comes from unknown sources like external memory and integer cast), the
+  // situation becomes a bit more interesting. We follow three general rules
----------------
"like external memory or an integer cast"


http://reviews.llvm.org/D21110