[PATCH] D59898: [LCG] Add aliased functions as LCG roots

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

Good catch!



================
Comment at: lib/Analysis/LazyCallGraph.cpp:191
+
+  // Functions has alias are also reachable.
+  for (auto &A : M.aliases()) {
----------------
I would rewrite the comment to more closely match the patch description: "Externally visible aliases of internal functions are also viable entry edges to the module."


================
Comment at: lib/Analysis/LazyCallGraph.cpp:192-200
+  for (auto &A : M.aliases()) {
+    if (A.hasLocalLinkage())
+      continue;
+    if (Function* F = dyn_cast<Function>(A.getAliasee())) {
+      LLVM_DEBUG(dbgs() << "  Adding '" << F->getName()
+                        << "' to entry set of the graph.\n");
+      addEdge(EntryEdges.Edges, EntryEdges.EdgeIndexMap, get(*F), Edge::Ref);
----------------
All of this should go above I think, right after we walk the module. We want to have the complete entry edge set before we visit globals and do the reference visit walk.


================
Comment at: lib/Analysis/LazyCallGraph.cpp:196-197
+    if (Function* F = dyn_cast<Function>(A.getAliasee())) {
+      LLVM_DEBUG(dbgs() << "  Adding '" << F->getName()
+                        << "' to entry set of the graph.\n");
+      addEdge(EntryEdges.Edges, EntryEdges.EdgeIndexMap, get(*F), Edge::Ref);
----------------
Include the fact that it is being added due to an alias, and the name of that alias?


================
Comment at: test/Analysis/LazyCallGraph/alias.ll:3
+;
+; Aliased function should be reachable in CGSCC.
+
----------------
Also test that internal aliases *don't* become reachable?


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

https://reviews.llvm.org/D59898