[PATCH] D22080: [CFLAA] Simplify CFLGraphBuilder by removing InstantiatedRelations and InstantiatedAttrs

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 11 15:21:59 PDT 2016


george.burgess.iv accepted this revision.
george.burgess.iv added a comment.
This revision is now accepted and ready to land.

LGTM after a few more nits. Thanks again!


================
Comment at: lib/Analysis/CFLGraph.h:99
@@ +98,3 @@
+    auto &ValInfo = ValueImpls[N.Val];
+    auto changed = ValInfo.addNodeToLevel(N.DerefLevel);
+    ValInfo.getNodeInfoAtLevel(N.DerefLevel).Attr |= Attr;
----------------
s/changed/Changed/ (aren't naming conventions fun?)

================
Comment at: lib/Analysis/CFLGraph.h:180
@@ -205,14 +179,3 @@
 
-    void addNode(Value *Val) {
-      assert(Val != nullptr);
-      if (!Graph.addNode(Val))
-        return;
-
-      if (isa<GlobalValue>(Val)) {
-        Graph.addAttr(Val, getGlobalOrArgAttrFromValue(*Val));
-        // Currently we do not attempt to be smart on globals
-        InstantiatedAttrs.push_back(
-            InstantiatedAttr{InstantiatedValue{Val, 1}, getAttrUnknown()});
-      } else if (auto CExpr = dyn_cast<ConstantExpr>(Val))
-        if (hasUsefulEdges(CExpr))
-          visitConstantExpr(CExpr);
+    void checkGlobalOrCExpr(Value *Val) {}
+
----------------
Please remove this function

================
Comment at: lib/Analysis/CFLGraph.h:490
@@ +489,3 @@
+      Graph.addNode(InstantiatedValue{&Arg, 0});
+      Graph.addAttr(InstantiatedValue{&Arg, 0},
+                    getGlobalOrArgAttrFromValue(Arg));
----------------
Please swap this to `addNode(value, attr)`

================
Comment at: lib/Analysis/CFLGraph.h:494
@@ -512,1 +493,3 @@
+      Graph.addNode(InstantiatedValue{&Arg, 1});
+      Graph.addAttr(InstantiatedValue{&Arg, 1}, getAttrCaller());
     }
----------------
This, too


http://reviews.llvm.org/D22080





More information about the llvm-commits mailing list