Skip to content

[DebugInfo][RemoveDIs] Erase ranges of instructions individually #81007

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
Feb 8, 2024

Conversation

jmorse
Copy link
Member

@jmorse jmorse commented Feb 7, 2024

The BasicBlock::erase method simply removes a range of instructions from the instlist by unlinking them. However, now that we're attaching debug-info directly to instructions, some cleanup is required, so use eraseFromParent on each instruction instead.

This is less efficient, but rare, and seemingly only WASM EH Prepare uses this method of BasicBlock. Detected via a memory leak check in asan.

(asan is always the final boss for whatever I do).

The BasicBlock::erase method simply removes a range of instructions from
the instlist by unlinking them. However, now that we're attaching
debug-info directly to instructions, some cleanup is required, so use
eraseFromParent on each instruction instead.

This is less efficient, but rare, and seemingly only WASM EH Prepare uses
this method of BasicBlock. Detected via a memory leak check in asan.
@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-debuginfo

Author: Jeremy Morse (jmorse)

Changes

The BasicBlock::erase method simply removes a range of instructions from the instlist by unlinking them. However, now that we're attaching debug-info directly to instructions, some cleanup is required, so use eraseFromParent on each instruction instead.

This is less efficient, but rare, and seemingly only WASM EH Prepare uses this method of BasicBlock. Detected via a memory leak check in asan.

(asan is always the final boss for whatever I do).


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

1 Files Affected:

  • (modified) llvm/lib/IR/BasicBlock.cpp (+3-1)
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index dca5283283847..0f89a8d8c8928 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -675,7 +675,9 @@ BasicBlock *BasicBlock::splitBasicBlockBefore(iterator I, const Twine &BBName) {
 
 BasicBlock::iterator BasicBlock::erase(BasicBlock::iterator FromIt,
                                        BasicBlock::iterator ToIt) {
-  return InstList.erase(FromIt, ToIt);
+  for (Instruction &I : make_early_inc_range(make_range(FromIt, ToIt)))
+    I.eraseFromParent();
+  return ToIt;
 }
 
 void BasicBlock::replacePhiUsesWith(BasicBlock *Old, BasicBlock *New) {

@jmorse jmorse merged commit 1a42b38 into llvm:main Feb 8, 2024
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