Skip to content

Commit

Permalink
[MCP] Do not try forward non-existent sub-register of a copy
Browse files Browse the repository at this point in the history
In this example:
```
$d14 = COPY killed $d18
$s0 = MI $s28
```

$s28 is a sub-register of $d14. However, $d18 does not have
sub-registers and thus cannot be forwarded. Previously, this resulted
in $noreg being substituted in place of the use of $s28, which later
led to an assertion failure.

Fixes #60908, a regression
that was introduced in D141747.

Reviewed By: arsenm

Differential Revision: https://reviews.llvm.org/D146930
  • Loading branch information
s-barannikov committed Mar 30, 2023
1 parent 50f2ce4 commit 1f5e9a3
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 12 deletions.
25 changes: 13 additions & 12 deletions llvm/lib/CodeGen/MachineCopyPropagation.cpp
Expand Up @@ -643,15 +643,19 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
const MachineOperand &CopySrc = *CopyOperands->Source;
Register CopySrcReg = CopySrc.getReg();

// When the use is a subregister of the COPY destination,
// record the subreg index.
unsigned SubregIdx = 0;

// This can only occur when we are dealing with physical registers.
Register ForwardedReg = CopySrcReg;
// MI might use a sub-register of the Copy destination, in which case the
// forwarded register is the matching sub-register of the Copy source.
if (MOUse.getReg() != CopyDstReg) {
SubregIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
if (!SubregIdx)
unsigned SubRegIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
assert(SubRegIdx &&
"MI source is not a sub-register of Copy destination");
ForwardedReg = TRI->getSubReg(CopySrcReg, SubRegIdx);
if (!ForwardedReg) {
LLVM_DEBUG(dbgs() << "MCP: Copy source does not have sub-register "
<< TRI->getSubRegIndexName(SubRegIdx) << '\n');
continue;
}
}

// Don't forward COPYs of reserved regs unless they are constant.
Expand Down Expand Up @@ -681,13 +685,10 @@ void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
}

LLVM_DEBUG(dbgs() << "MCP: Replacing " << printReg(MOUse.getReg(), TRI)
<< "\n with " << printReg(CopySrcReg, TRI)
<< "\n with " << printReg(ForwardedReg, TRI)
<< "\n in " << MI << " from " << *Copy);

if (SubregIdx)
MOUse.setReg(TRI->getSubReg(CopySrcReg, SubregIdx));
else
MOUse.setReg(CopySrcReg);
MOUse.setReg(ForwardedReg);

if (!CopySrc.isRenamable())
MOUse.setIsRenamable(false);
Expand Down
45 changes: 45 additions & 0 deletions llvm/test/CodeGen/ARM/pr60908.mir
@@ -0,0 +1,45 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 2
# RUN: llc -mtriple=armv7 -run-pass=machine-cp %s -o - | FileCheck %s

# Positive test: $d15 has sub-register $s30, which should be propagated.
---
name: test_d15
tracksRegLiveness: true
liveins:
- { reg: '$d15' }
body: |
bb.0.entry:
liveins: $d15
; CHECK-LABEL: name: test_d15
; CHECK: liveins: $d15
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $s0 = COPY $s30
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
renamable $d14 = COPY killed $d15
$s0 = COPY killed renamable $s28
BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
...

# Negative test: $d18 does not have sub-registers.
---
name: test_d18
tracksRegLiveness: true
liveins:
- { reg: '$d18' }
body: |
bb.0.entry:
liveins: $d18
; CHECK-LABEL: name: test_d18
; CHECK: liveins: $d18
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: renamable $d14 = COPY killed $d18
; CHECK-NEXT: $s0 = COPY killed renamable $s28
; CHECK-NEXT: BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
renamable $d14 = COPY killed $d18
$s0 = COPY killed renamable $s28
BX_RET 14 /* CC::al */, $noreg, implicit killed $s0
...

0 comments on commit 1f5e9a3

Please sign in to comment.