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

[George Burgess IV via llvm-commits]

george.burgess.iv added a comment.

Looks good with a few nits/comments. Thanks for the patch!


================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1024
@@ +1023,3 @@
+      // TODO: do we need to filter out non-pointer values here?
+      Builder.addAttributesBelow(&Val, AttrUnknown);
+    }
----------------
If there's no set below `Val` when calling `addAttributesBelow`, do we need to create one just to tag it with attributes? If not, maybe we could add a `forceCreation` flag (that defaults to true) or something?

================
Comment at: lib/Analysis/CFLAliasAnalysis.cpp:1030
@@ +1029,3 @@
+    ProcessGlobalOrArgValue(Arg);
+  for (auto Global : Globals)
+    ProcessGlobalOrArgValue(*Global);
----------------
Style nit: It's generally encouraged to do `auto *` if the value is a pointer. (Though, I'm not sure that the rest of the file follows this perfectly)

================
Comment at: lib/Analysis/StratifiedSets.h:393
@@ -392,1 +392,3 @@
 
+  /// \breif Set the StratifiedAttrs of the set below "Main". If there is no set
+  /// below "Main", create one for it.
----------------
breif -> brief

================
Comment at: test/Analysis/CFLAliasAnalysis/attr-escape.ll:6
@@ -5,5 +5,3 @@
 
-; CHECK: Function: escape_ptrtoint
-; CHECK: NoAlias: i32* %a, i32* %x
-; CHECK: NoAlias: i32* %b, i32* %x
+; CHECK: Function: test_local
 ; CHECK: NoAlias: i32* %a, i32* %b
----------------
Nit that I should've caught earlier: CHECK-LABEL should be used for functions. This lets FileCheck check the remainder of functions in the file if there's a problem with this one (for whatever reason).

================
Comment at: test/Analysis/CFLAliasAnalysis/attr-escape.ll:18
@@ +17,3 @@
+
+; CHECK: Function: test_global_param
+; CHECK: NoAlias: i32* %a, i32** %x
----------------
CHECK-LABEL


http://reviews.llvm.org/D21110