Skip to content

[MemProf] Stop cloning traversal on single allocation type #126131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

teresajohnson
Copy link
Contributor

We were previously checking this after recursing on all callers, but if
we already have a single allocation type there is no need to even look
at any callers. Didn't show a significant improvement overall, but it
does reduce the count of times we enter the identifyClones and do other
checks.

We were previously checking this after recursing on all callers, but if
we already have a single allocation type there is no need to even look
at any callers. Didn't show a significant improvement overall, but it
does reduce the count of times we enter the identifyClones and do other
checks.
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Teresa Johnson (teresajohnson)

Changes

We were previously checking this after recursing on all callers, but if
we already have a single allocation type there is no need to even look
at any callers. Didn't show a significant improvement overall, but it
does reduce the count of times we enter the identifyClones and do other
checks.


Full diff: https://github.com/llvm/llvm-project/pull/126131.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp (+4)
diff --git a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
index f6764be9b273e7..d748b162d78094 100644
--- a/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
+++ b/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp
@@ -3407,6 +3407,10 @@ void CallsiteContextGraph<DerivedCCG, FuncTy, CallTy>::identifyClones(
   if (!Node->hasCall())
     return;
 
+  // No need to look at any callers if allocation type already unambiguous.
+  if (hasSingleAllocType(Node->AllocTypes))
+    return;
+
 #ifndef NDEBUG
   auto Insert =
 #endif

@teresajohnson teresajohnson merged commit 1dbfbb5 into llvm:main Feb 6, 2025
7 of 9 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
We were previously checking this after recursing on all callers, but if
we already have a single allocation type there is no need to even look
at any callers. Didn't show a significant improvement overall, but it
does reduce the count of times we enter the identifyClones and do other
checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants