[TSan] Fix missing inst cleanup

[Camsyn]

Commit 44e875a introduced a change that replaces ReplaceInstWithInst with Instruction::replaceAllUsesWith, without subsequent instruction cleanup.

This results in TSan leaving behind useless load atomic instructions after 'replacing' them.

This commit adds cleanup back, consistent with the context.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Kunqiu Chen (Camsyn)

Changes

Commit 44e875a introduced a change that replaces ReplaceInstWithInst with Instruction::replaceAllUsesWith, without subsequent instruction cleanup.

This results in TSan leaving behind useless load atomic instructions after 'replacing' them.

This commit adds cleanup back, consistent with the context.

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp (+1)

diff --git a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
index ec9f78edfeb1c..195a157ee20af 100644
--- a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -729,6 +729,7 @@ bool ThreadSanitizer::instrumentAtomic(Instruction *I, const DataLayout &DL) {
     Value *C = IRB.CreateCall(TsanAtomicLoad[Idx], Args);
     Value *Cast = IRB.CreateBitOrPointerCast(C, OrigTy);
     I->replaceAllUsesWith(Cast);
+    I->eraseFromParent();
   } else if (StoreInst *SI = dyn_cast<StoreInst>(I)) {
     Value *Addr = SI->getPointerOperand();
     int Idx =

[nikic]

Can you please rebase over 4f8187c and regenerate the test, so we can see the difference?

[Camsyn]

Can you please rebase over 4f8187c and regenerate the test, so we can see the difference?

Done. It just removes some useless load atomics, which should have been replaced by call __tsan_atomic*_load.
Friendly ping @nikic .

[nikic]

LGTM