[llvm] [RISCV] Move RISCVInsertVSETVLI to after phi elimination (PR #91440)

Min-Yih Hsu via llvm-commits llvm-commits at lists.llvm.org
Wed May 15 09:59:15 PDT 2024


================
@@ -541,9 +541,16 @@ void RISCVPassConfig::addPreRegAlloc() {
   addPass(createRISCVPreRAExpandPseudoPass());
   if (TM->getOptLevel() != CodeGenOptLevel::None)
     addPass(createRISCVMergeBaseOffsetOptPass());
+
   addPass(createRISCVInsertReadWriteCSRPass());
   addPass(createRISCVInsertWriteVXRMPass());
-  addPass(createRISCVInsertVSETVLIPass());
+
+  // 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());
----------------
mshockwave wrote:

I think the `insertPass` here _might_ have made ASAN buildbot unhappy:
```
******************** TEST 'LLVM :: CodeGen/RISCV/GlobalISel/irtranslator/calling-conv-lp64-lp64f-common.ll' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llc -mtriple=riscv64      -global-isel -stop-after=irtranslator -verify-machineinstrs < /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calling-conv-lp64-lp64f-common.ll    | /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck -check-prefixes=RV64I,LP64 /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calling-conv-lp64-lp64f-common.ll
+ /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/llc -mtriple=riscv64 -global-isel -stop-after=irtranslator -verify-machineinstrs
+ /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm_build_asan/bin/FileCheck -check-prefixes=RV64I,LP64 /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/test/CodeGen/RISCV/GlobalISel/irtranslator/calling-conv-lp64-lp64f-common.ll

=================================================================
==3105773==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 352 byte(s) in 1 object(s) allocated from:
   #0 0x55c96a0ec1ed in operator new(unsigned long) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:86:3
   #1 0x55c96ccb903f in llvm::createRISCVInsertVSETVLIPass() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp:1812:10
   #2 0x55c96cc78e72 in (anonymous namespace)::RISCVPassConfig::addPreRegAlloc() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
   #3 0x55c96f565e73 in llvm::TargetPassConfig::addMachinePasses() /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/CodeGen/TargetPassConfig.cpp:1114:3
   #4 0x55c96ebbc578 in addPassesToGenerateCode /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/CodeGen/LLVMTargetMachine.cpp:125:15
   #5 0x55c96ebbc578 in llvm::LLVMTargetMachine::addPassesToEmitFile(llvm::legacy::PassManagerBase&, llvm::raw_pwrite_stream&, llvm::raw_pwrite_stream*, llvm::CodeGenFileType, bool, llvm::MachineModuleInfoWrapperPass*) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/lib/CodeGen/LLVMTargetMachine.cpp:237:7
   #6 0x55c96a102093 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llc/llc.cpp:710:24
   #7 0x55c96a0fd61b in main /b/sanitizer-x86_64-linux-bootstrap-asan/build/llvm-project/llvm/tools/llc/llc.cpp:408:22
   #8 0x7f3ece42814f  (/lib/x86_64-linux-gnu/libc.so.6+0x2814f) (BuildId: b20cbdb62d7717c13dc61a48b7b2e673a7edf233)

SUMMARY: AddressSanitizer: 352 byte(s) leaked in 1 allocation(s).
```
Here is my thoughts on what happened:
  1. The `Pass *` created here is eventually stored in `PassConfigImpl::InsertedPass`
  2. In `TargetPassConfig::addPass`, it looks up `InsertedPass` and insert a Pass (in this case RISCVInsertVSETVLI) at the desired location (after PHIElimination in this case) by calling `addPass` recursively
  3. However, inside this recursion if in any case you reach the cutting point set up by `-stop-after`, `addPass` actually `delete` the `Pass *` instance.
  4. ...But `InsertedPass` still holds the now-dangling `Pass *` pointer.

I think the above scenario could definitely happen. What doesn't really make sense is the command reported by buildbot is calling with `-stop-after=irtranslator` which seems irrelevant.

Note: all the other `insertPass` usages are called with PassID like `insertPass(&TwoAddressInstructionPassID, &SIWholeQuadModeID);` rather than called with Pass pointer.

https://github.com/llvm/llvm-project/pull/91440


More information about the llvm-commits mailing list