Skip to content

Commit

Permalink
MachineCopyPropagation: Do not remove copies preserved by regmask (ll…
Browse files Browse the repository at this point in the history
…vm#125868)

llvm/llvm-project@9e436c2daa44 tries to handle register masks and
sub-registers, it avoids clobbering RegUnit presreved by regmask. But it
then introduces invalid pointer issues.

We delete the copies without invalidate all the use in the CopyInfo, so
we dereferenced invalid pointers in next interation, causing asserts.

Fixes: llvm#126107

---------

Co-authored-by: Matt Arsenault <[email protected]>
  • Loading branch information
jsji and arsenm authored Feb 10, 2025
1 parent 0a470a9 commit 5d2e284
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 4 deletions.
20 changes: 16 additions & 4 deletions llvm/lib/CodeGen/MachineCopyPropagation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1018,18 +1018,30 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
continue;
}

LLVM_DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: ";
MaybeDead->dump());

// Invalidate all entries in the copy map which are not preserved by
// this register mask.
for (unsigned RegUnit : TRI->regunits(Reg))
bool MIRefedinCopyInfo = false;
for (unsigned RegUnit : TRI->regunits(Reg)) {
if (!PreservedRegUnits.test(RegUnit))
Tracker.clobberRegUnit(RegUnit, *TRI, *TII, UseCopyInstr);
else {
if (MaybeDead == Tracker.findCopyForUnit(RegUnit, *TRI)) {
MIRefedinCopyInfo = true;
}
}
}

// erase() will return the next valid iterator pointing to the next
// element after the erased one.
DI = MaybeDeadCopies.erase(DI);

// Preserved by RegMask, DO NOT remove copy
if (MIRefedinCopyInfo)
continue;

LLVM_DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: "
<< *MaybeDead);

MaybeDead->eraseFromParent();
Changed = true;
++NumDeletes;
Expand Down
17 changes: 17 additions & 0 deletions llvm/test/CodeGen/MIR/X86/pr126107.mir
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -o - %s -mtriple=x86_64-- -run-pass=machine-cp | FileCheck %s

---
name: main
body: |
bb.0.entry:
liveins: $ymm7
; CHECK-LABEL: name: main
; CHECK: liveins: $ymm7
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $ymm6 = COPY killed renamable $ymm7
; CHECK-NEXT: CALL64r killed renamable $rax, csr_64_mostregs
; CHECK-NEXT: renamable $ymm6 = VPADDWZ256rr $ymm6, $ymm6
renamable $ymm6 = COPY killed renamable $ymm7
CALL64r killed renamable $rax, csr_64_mostregs
renamable $ymm6 = VPADDWZ256rr $ymm6, $ymm6

0 comments on commit 5d2e284

Please sign in to comment.