Skip to content

[PredicateInfo] Cache ssa.copy declarations (NFC) #145020

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
merged 2 commits into from
Jun 23, 2025

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Jun 20, 2025

This pass creates a lot of ssa.copy intrinsics, typically for a small set of types. Determining the function type, performing intrinsic name mangling and looking up the declaration has noticeable overhead in this case.

Improve this by caching the declarations by type. I've made this a separate map from CreatedDeclarations, which only tracks the declarations that were newly inserted (but not pre-existing ones).

This pass creates a lot of ssa.copy intrinsics, typically for a
small set of types. Determining the function type, performing
intrinsic name mangling and looking up the declaration has
noticeable overhead in this case.

Improve this by caching the declarations by type. I've made this
a separate map from CreatedDeclarations, which only tracks the
declarations that were newly inserted (but not pre-existing ones).
@nikic nikic requested review from fhahn and dtcxzyw June 20, 2025 11:08
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

This pass creates a lot of ssa.copy intrinsics, typically for a small set of types. Determining the function type, performing intrinsic name mangling and looking up the declaration has noticeable overhead in this case.

Improve this by caching the declarations by type. I've made this a separate map from CreatedDeclarations, which only tracks the declarations that were newly inserted (but not pre-existing ones).


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

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Utils/PredicateInfo.h (+2)
  • (modified) llvm/lib/Transforms/Utils/PredicateInfo.cpp (+15-12)
diff --git a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h
index 670bfaa5ad6fe..1619fa31fb6f4 100644
--- a/llvm/include/llvm/Transforms/Utils/PredicateInfo.h
+++ b/llvm/include/llvm/Transforms/Utils/PredicateInfo.h
@@ -206,6 +206,8 @@ class PredicateInfo {
   DenseMap<const Value *, const PredicateBase *> PredicateMap;
   // The set of ssa_copy declarations we created with our custom mangling.
   SmallSet<AssertingVH<Function>, 20> CreatedDeclarations;
+  // Cache of ssa.copy declaration for a given type.
+  SmallDenseMap<Type *, Function *> DeclarationCache;
 };
 
 /// Printer pass for \c PredicateInfo.
diff --git a/llvm/lib/Transforms/Utils/PredicateInfo.cpp b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
index 97f13e3b26746..074b87265e375 100644
--- a/llvm/lib/Transforms/Utils/PredicateInfo.cpp
+++ b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
@@ -499,6 +499,19 @@ Value *PredicateInfoBuilder::materializeStack(unsigned int &Counter,
     ValInfo->RenamedOp = (RenameStack.end() - Start) == RenameStack.begin()
                              ? OrigOp
                              : (RenameStack.end() - Start - 1)->Def;
+    auto CreateSSACopy = [this](IRBuilderBase &B, Value *Op,
+                                const Twine &Name = "") {
+      auto It = PI.DeclarationCache.try_emplace(Op->getType());
+      if (It.second) {
+        auto NumDecls = F.getParent()->getNumNamedValues();
+        Function *IF = Intrinsic::getOrInsertDeclaration(
+            F.getParent(), Intrinsic::ssa_copy, Op->getType());
+        if (NumDecls != F.getParent()->getNumNamedValues())
+          PI.CreatedDeclarations.insert(IF);
+        It.first->second = IF;
+      }
+      return B.CreateCall(It.first->second, Op, Name);
+    };
     // For edge predicates, we can just place the operand in the block before
     // the terminator. For assume, we have to place it right after the assume
     // to ensure we dominate all uses except assume itself. Always insert
@@ -511,13 +524,8 @@ Value *PredicateInfoBuilder::materializeStack(unsigned int &Counter,
     // represents an invalid module.
     if (isa<PredicateWithEdge>(ValInfo)) {
       IRBuilder<> B(getBranchTerminator(ValInfo));
-      auto NumDecls = F.getParent()->getNumNamedValues();
-      Function *IF = Intrinsic::getOrInsertDeclaration(
-          F.getParent(), Intrinsic::ssa_copy, Op->getType());
-      if (NumDecls != F.getParent()->getNumNamedValues())
-        PI.CreatedDeclarations.insert(IF);
       CallInst *PIC =
-          B.CreateCall(IF, Op, Op->getName() + "." + Twine(Counter++));
+          CreateSSACopy(B, Op, Op->getName() + "." + Twine(Counter++));
       PI.PredicateMap.insert({PIC, ValInfo});
       Result.Def = PIC;
     } else {
@@ -527,12 +535,7 @@ Value *PredicateInfoBuilder::materializeStack(unsigned int &Counter,
       // Insert the predicate directly after the assume. While it also holds
       // directly before it, assume(i1 true) is not a useful fact.
       IRBuilder<> B(PAssume->AssumeInst->getNextNode());
-      auto NumDecls = F.getParent()->getNumNamedValues();
-      Function *IF = Intrinsic::getOrInsertDeclaration(
-          F.getParent(), Intrinsic::ssa_copy, Op->getType());
-      if (NumDecls != F.getParent()->getNumNamedValues())
-        PI.CreatedDeclarations.insert(IF);
-      CallInst *PIC = B.CreateCall(IF, Op);
+      CallInst *PIC = CreateSSACopy(B, Op);
       PI.PredicateMap.insert({PIC, ValInfo});
       Result.Def = PIC;
     }

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@nikic nikic merged commit 86beba9 into llvm:main Jun 23, 2025
7 checks passed
@nikic nikic deleted the predicate-info-decl-cache branch June 23, 2025 07:16
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jun 23, 2025

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/25070

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/altered_threadState.test (3035 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/app_specific_backtrace_crashlog.test (3036 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/interactive_crashlog_json.test (3037 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/skipped_status_interactive_crashlog.test (3038 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/io.test (3039 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/nested_sessions.test (3040 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/independent_state.test (3041 of 3046)
UNSUPPORTED: lldb-shell :: Expr/TestIRMemoryMapWindows.test (3042 of 3046)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/no_threadState.test (3043 of 3046)
UNRESOLVED: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py (3044 of 3046)
******************** TEST 'lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib --cmake-build-type Release -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch -p TestDAP_launch.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 86beba9301112c6092cbfa3e53bdacc0d68337df)
  clang revision 86beba9301112c6092cbfa3e53bdacc0d68337df
  llvm revision 86beba9301112c6092cbfa3e53bdacc0d68337df
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch
runCmd: settings clear --all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 

runCmd: settings set target.auto-apply-fixits false

miguelcsx pushed a commit to miguelcsx/llvm-project that referenced this pull request Jun 23, 2025
This pass creates a lot of ssa.copy intrinsics, typically for a small
set of types. Determining the function type, performing intrinsic name
mangling and looking up the declaration has noticeable overhead in this
case.

Improve this by caching the declarations by type. I've made this a
separate map from CreatedDeclarations, which only tracks the
declarations that were newly inserted (but not pre-existing ones).
Jaddyen pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 23, 2025
This pass creates a lot of ssa.copy intrinsics, typically for a small
set of types. Determining the function type, performing intrinsic name
mangling and looking up the declaration has noticeable overhead in this
case.

Improve this by caching the declarations by type. I've made this a
separate map from CreatedDeclarations, which only tracks the
declarations that were newly inserted (but not pre-existing ones).
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.

5 participants