[llvm] RegisterCoalescer: Fix producing malformed IMPLICIT_DEFs (PR #73784)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 2 06:58:18 PST 2023
https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/73784
>From 70b843ae31ff42a9ece8af5a5b5dba9e526ab4c9 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Tue, 28 Nov 2023 17:49:34 +0900
Subject: [PATCH 1/2] RegisterCoalescer: Fix producing malformed IMPLICIT_DEFs
If this was coalescing a SUBREG_TO_REG as a copy, the resulting
instruction would be an IMPLICIT_DEF with an unexpected 2 immediate
operands, which need to be dropped. Until recently the verifier
did not catch this error, and an assert would fire if later
the broken IMPLICIT_DEF was rematerialized.
PR#73758 is related, it changes the failure mode to a verifier error.
---
llvm/lib/CodeGen/RegisterCoalescer.cpp | 11 +-
.../coalescer-drop-subreg-to-reg-imm-ops.mir | 120 ++++++++++++++++++
2 files changed, 129 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/CodeGen/AArch64/coalescer-drop-subreg-to-reg-imm-ops.mir
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index a55f3941c98b3..2c421fa452ad7 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1694,12 +1694,19 @@ MachineInstr *RegisterCoalescer::eliminateUndefCopy(MachineInstr *CopyMI) {
// The source interval may also have been on an undef use, in which case the
// copy introduced a live value.
if (((V && V->isPHIDef()) || (!V && !DstLI.liveAt(Idx)))) {
- CopyMI->setDesc(TII->get(TargetOpcode::IMPLICIT_DEF));
for (unsigned i = CopyMI->getNumOperands(); i != 0; --i) {
MachineOperand &MO = CopyMI->getOperand(i-1);
- if (MO.isReg() && MO.isUse())
+ if (MO.isReg()) {
+ if (MO.isUse())
+ CopyMI->removeOperand(i - 1);
+ } else {
+ assert(MO.isImm() &&
+ (CopyMI->getOpcode() == TargetOpcode::SUBREG_TO_REG));
CopyMI->removeOperand(i-1);
+ }
}
+
+ CopyMI->setDesc(TII->get(TargetOpcode::IMPLICIT_DEF));
LLVM_DEBUG(dbgs() << "\tReplaced copy of <undef> value with an "
"implicit def\n");
return CopyMI;
diff --git a/llvm/test/CodeGen/AArch64/coalescer-drop-subreg-to-reg-imm-ops.mir b/llvm/test/CodeGen/AArch64/coalescer-drop-subreg-to-reg-imm-ops.mir
new file mode 100644
index 0000000000000..f54c612303c0e
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/coalescer-drop-subreg-to-reg-imm-ops.mir
@@ -0,0 +1,120 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
+# RUN: llc -mtriple=arm64-apple-macosx -mcpu=apple-m1 -verify-coalescing -run-pass=register-coalescer -o - %s | FileCheck %s
+
+# Hits assert "Trying to add an operand to a machine instr that is
+# already done!" when rematerializing during greedy. This was because
+# an IMPLICIT_DEF ended up with some immediate operands during
+# coalescing. A SUBREG_TO_REG was not dropping the immediate operands
+# when mutating to IMPLICIT_DEF, and would later fail the assert when
+# creating a new IMPLICIT_DEF copy during rematerialization.
+
+--- |
+ define void @_ZN38SanitizerCommonInterceptors_Scanf_Test8TestBodyEv() {
+ ret void
+ }
+
+ declare void @_ZL9testScanfPKcjz(ptr, i32, ...)
+
+...
+---
+name: _ZN38SanitizerCommonInterceptors_Scanf_Test8TestBodyEv
+alignment: 4
+tracksRegLiveness: true
+frameInfo:
+ maxAlignment: 8
+ adjustsStack: true
+ hasCalls: true
+ maxCallFrameSize: 24
+body: |
+ bb.0:
+ liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6
+
+ ; CHECK-LABEL: name: _ZN38SanitizerCommonInterceptors_Scanf_Test8TestBodyEv
+ ; CHECK: liveins: $x0, $x1, $x2, $x3, $x4, $x5, $x6
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[DEF:%[0-9]+]]:gpr64sp = IMPLICIT_DEF
+ ; CHECK-NEXT: dead [[DEF1:%[0-9]+]]:gpr32 = IMPLICIT_DEF
+ ; CHECK-NEXT: [[DEF2:%[0-9]+]]:gpr64common = IMPLICIT_DEF
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr64 = COPY $x5
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:gpr64 = COPY $x4
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:gpr64 = COPY $x3
+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr64 = COPY $x2
+ ; CHECK-NEXT: [[COPY4:%[0-9]+]]:gpr64 = COPY $x1
+ ; CHECK-NEXT: [[COPY5:%[0-9]+]]:gpr64 = COPY $x0
+ ; CHECK-NEXT: [[DEF3:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+ ; CHECK-NEXT: [[DEF4:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+ ; CHECK-NEXT: [[DEF5:%[0-9]+]]:gpr64 = IMPLICIT_DEF
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 16, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: BL @_ZL9testScanfPKcjz, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+ ; CHECK-NEXT: ADJCALLSTACKUP 16, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: STRXui [[DEF3]], [[DEF]], 0 :: (store (s64) into stack)
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: STRWui undef [[DEF1]], [[DEF2]], 0 :: (store (s32))
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: STRXui [[DEF4]], undef [[DEF]], 0 :: (store (s64) into stack)
+ ; CHECK-NEXT: $x0 = COPY [[COPY5]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: $x0 = COPY [[COPY4]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: $x0 = COPY [[COPY3]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: $x0 = COPY [[COPY2]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: $x0 = COPY [[COPY1]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: $x0 = COPY [[COPY]]
+ ; CHECK-NEXT: ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: ADJCALLSTACKDOWN 24, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: STRXui [[DEF5]], undef [[DEF]], 1 :: (store (s64) into stack + 8)
+ ; CHECK-NEXT: ADJCALLSTACKUP 24, 0, implicit-def dead $sp, implicit $sp
+ ; CHECK-NEXT: RET_ReallyLR
+ %0:gpr64sp = IMPLICIT_DEF
+ %1:gpr32 = IMPLICIT_DEF
+ %2:gpr64common = IMPLICIT_DEF
+ %3:gpr64 = COPY killed $x5
+ %4:gpr64 = COPY killed $x4
+ %5:gpr64 = COPY killed $x3
+ %6:gpr64 = COPY killed $x2
+ %7:gpr64 = COPY killed $x1
+ %8:gpr64 = COPY killed $x0
+ %9:gpr64 = IMPLICIT_DEF
+ %10:gpr64 = IMPLICIT_DEF
+ %11:gpr64 = SUBREG_TO_REG 0, killed undef %1, %subreg.sub_32
+ ADJCALLSTACKDOWN 16, 0, implicit-def dead $sp, implicit $sp
+ BL @_ZL9testScanfPKcjz, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+ ADJCALLSTACKUP 16, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ STRXui %9, killed %0, 0 :: (store (s64) into stack)
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ STRWui undef %1, killed %2, 0 :: (store (s32))
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ STRXui killed %10, killed undef %0, 0 :: (store (s64) into stack)
+ $x0 = COPY killed %8
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ $x0 = COPY killed %7
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ $x0 = COPY killed %6
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ $x0 = COPY killed %5
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ $x0 = COPY killed %4
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 8, 0, implicit-def dead $sp, implicit $sp
+ $x0 = COPY killed %3
+ ADJCALLSTACKUP 8, 0, implicit-def dead $sp, implicit $sp
+ ADJCALLSTACKDOWN 24, 0, implicit-def dead $sp, implicit $sp
+ STRXui killed %11, undef %0, 1 :: (store (s64) into stack + 8)
+ ADJCALLSTACKUP 24, 0, implicit-def dead $sp, implicit $sp
+ RET_ReallyLR
+
+...
>From 89b6bbced1d3e6d7d04aed6d703708b11744b0e5 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Sat, 2 Dec 2023 21:58:01 +0700
Subject: [PATCH 2/2] Fix extra parens
---
llvm/lib/CodeGen/RegisterCoalescer.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 2c421fa452ad7..c067d87a9fd81 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1701,7 +1701,7 @@ MachineInstr *RegisterCoalescer::eliminateUndefCopy(MachineInstr *CopyMI) {
CopyMI->removeOperand(i - 1);
} else {
assert(MO.isImm() &&
- (CopyMI->getOpcode() == TargetOpcode::SUBREG_TO_REG));
+ CopyMI->getOpcode() == TargetOpcode::SUBREG_TO_REG);
CopyMI->removeOperand(i-1);
}
}
More information about the llvm-commits
mailing list