[PATCH] D140908: [MemProf] Context disambiguation cloning pass [patch 1/3]

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Updating again with another fix from my testing (see description below), along with a new test for it. I will be addressing the remaining comments/suggestions next.



================
Comment at: llvm/lib/Transforms/IPO/PGHOContextDisambiguation.cpp:727
+      auto Inserted = Visited.insert(NextNode);
+      if (!Inserted.second)
+        continue;
----------------
I discovered when testing with a larger app and more graph validation enabled that this early return is not correct. We can have different subsets of duplicate context nodes being propagated along different edges to the same node, and so we were stopping the propagation early. This meant both an insane graph (context ids of node didn't match those of caller and callee edges) but also prevented some cloning. Removing this however leads to long compile time.

I redesigned the context duplication and the caller which is updateStackNodes to split this handling into 3:
1) walk the calls and perform all necessary context id duplication, saving the info in a map (in updateStackNodes and a modified duplicateContextIds).
2) propagate the context ids across the full graph in a single pass (in a new propagateDuplicateContextIds called from updateStackNodes).
3) do the post order traversal to generate new nodes for inlined call chains, moving the context ids determined earlier (which might be duplicates) to the new node (in updateStackNodes).

This allowed handling quite a few more cold calls in my large application, with smaller compile time than the patch even without the fix to remove this early termination. I'll upload the new version of this handling in a few minutes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140908