Skip to content

[RISCV] Defer creating RISCVInsertVSETVLI to avoid leak with -stop-after #92303

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 1 commit into from
May 16, 2024

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented May 15, 2024

As noted in #91440 (comment), if the pass pipeline stops early because of -stop-after any allocated passes added with insertPass will not be freed if they haven't already been added.

This was showing up as a failure on the address sanitizer buildbots. We can fix it by instead passing the pass ID instead so that allocation is deferred.

As noted in llvm#91440 (comment), if the pass pipeline stops early because of -stop-after any allocated passes added with insertPass will not be freed if they haven't already been added.

This was showing up as a failure on the address sanitizer buildbots. We can fix it by instead passing the pass ID instead so that allocation is deferred.
@llvmbot
Copy link
Member

llvmbot commented May 15, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Luke Lau (lukel97)

Changes

As noted in #91440 (comment), if the pass pipeline stops early because of -stop-after any allocated passes added with insertPass will not be freed if they haven't already been added.

This was showing up as a failure on the address sanitizer buildbots. We can fix it by instead passing the pass ID instead so that allocation is deferred.


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

3 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCV.h (+1)
  • (modified) llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp (+1)
  • (modified) llvm/lib/Target/RISCV/RISCVTargetMachine.cpp (+2-2)
diff --git a/llvm/lib/Target/RISCV/RISCV.h b/llvm/lib/Target/RISCV/RISCV.h
index d405395dcf9ec..2b8688c5de61f 100644
--- a/llvm/lib/Target/RISCV/RISCV.h
+++ b/llvm/lib/Target/RISCV/RISCV.h
@@ -60,6 +60,7 @@ void initializeRISCVExpandAtomicPseudoPass(PassRegistry &);
 
 FunctionPass *createRISCVInsertVSETVLIPass();
 void initializeRISCVInsertVSETVLIPass(PassRegistry &);
+extern char &RISCVInsertVSETVLIID;
 
 FunctionPass *createRISCVCoalesceVSETVLIPass();
 void initializeRISCVCoalesceVSETVLIPass(PassRegistry &);
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index 363007d7b68b1..324ce5cb5ed7d 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -868,6 +868,7 @@ class RISCVCoalesceVSETVLI : public MachineFunctionPass {
 } // end anonymous namespace
 
 char RISCVInsertVSETVLI::ID = 0;
+char &llvm::RISCVInsertVSETVLIID = RISCVInsertVSETVLI::ID;
 
 INITIALIZE_PASS(RISCVInsertVSETVLI, DEBUG_TYPE, RISCV_INSERT_VSETVLI_NAME,
                 false, false)
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 5d598a275a008..5aab138dae408 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -548,9 +548,9 @@ void RISCVPassConfig::addPreRegAlloc() {
   // Run RISCVInsertVSETVLI after PHI elimination. On O1 and above do it after
   // register coalescing so needVSETVLIPHI doesn't need to look through COPYs.
   if (TM->getOptLevel() == CodeGenOptLevel::None)
-    insertPass(&PHIEliminationID, createRISCVInsertVSETVLIPass());
+    insertPass(&PHIEliminationID, &RISCVInsertVSETVLIID);
   else
-    insertPass(&RegisterCoalescerID, createRISCVInsertVSETVLIPass());
+    insertPass(&RegisterCoalescerID, &RISCVInsertVSETVLIID);
 }
 
 void RISCVPassConfig::addFastRegAlloc() {

Copy link
Member

@mshockwave mshockwave 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

@lukel97 lukel97 merged commit 566fbb4 into llvm:main May 16, 2024
5 of 6 checks passed
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