[PATCH] D78861: [Attributor] [WIP] Track AA dependency using dependency graph

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I see there is nice progress. I left two comment wrt. to test. If you want me to



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:1483
+    return A.AllAbstractAttributes.front();
+  }
+
----------------
I agree that this will be a problem for unconnected graphs. How you organize the synthetic node is up to you. We need to make sure not to increase memory consumption but other than that we can move and replace the tiny Deps vectors, the AllAbstractAttribute vector, .. etc. as you see fit :)


================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:2975
 
+  const std::string getName() const override { return "AAMemoryLocation"; }
+
----------------
Can you add a comment to these overrides, just `/// See AbstractAttribute::getName()`.


================
Comment at: llvm/test/Transforms/Attributor/depgraph.ll:37
+  ret i32* %14
+}
+
----------------
Run mem2reg on this test case please.
Also add all 4 run lines used in other tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78861/new/

https://reviews.llvm.org/D78861