[llvm] d6f7f2a - Revert "[MachineCP] Correctly handle register masks and sub-registers (#122472)"
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 05:34:25 PST 2025
Author: Nikita Popov
Date: 2025-01-13T14:33:35+01:00
New Revision: d6f7f2a5fa0e305253f936cdc8364eecfd568121
URL: https://github.com/llvm/llvm-project/commit/d6f7f2a5fa0e305253f936cdc8364eecfd568121
DIFF: https://github.com/llvm/llvm-project/commit/d6f7f2a5fa0e305253f936cdc8364eecfd568121.diff
LOG: Revert "[MachineCP] Correctly handle register masks and sub-registers (#122472)"
This reverts commit e2a071ece58790f8dd4886e998033cab82e906fb.
This causes a large compile-time regression.
Added:
Modified:
llvm/lib/CodeGen/MachineCopyPropagation.cpp
llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index d2579e2d1b44cc..49ce4b660c3ae4 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -164,91 +164,67 @@ class CopyTracker {
Copies.erase(Unit);
}
- /// Clobber a single register unit, removing it from the tracker's copy maps.
- void clobberRegUnit(MCRegUnit Unit, const TargetRegisterInfo &TRI,
- const TargetInstrInfo &TII, bool UseCopyInstr) {
- auto I = Copies.find(Unit);
- if (I != Copies.end()) {
- // When we clobber the source of a copy, we need to clobber everything
- // it defined.
- markRegsUnavailable(I->second.DefRegs, TRI);
- // When we clobber the destination of a copy, we need to clobber the
- // whole register it defined.
- if (MachineInstr *MI = I->second.MI) {
- std::optional<DestSourcePair> CopyOperands =
- isCopyInstr(*MI, TII, UseCopyInstr);
-
- MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
- MCRegister Src = CopyOperands->Source->getReg().asMCReg();
-
- markRegsUnavailable(Def, TRI);
-
- // Since we clobber the destination of a copy, the semantic of Src's
- // "DefRegs" to contain Def is no longer effectual. We will also need
- // to remove the record from the copy maps that indicates Src defined
- // Def. Failing to do so might cause the target to miss some
- // opportunities to further eliminate redundant copy instructions.
- // Consider the following sequence during the
- // ForwardCopyPropagateBlock procedure:
- // L1: r0 = COPY r9 <- TrackMI
- // L2: r0 = COPY r8 <- TrackMI (Remove r9 defined r0 from tracker)
- // L3: use r0 <- Remove L2 from MaybeDeadCopies
- // L4: early-clobber r9 <- Clobber r9 (L2 is still valid in tracker)
- // L5: r0 = COPY r8 <- Remove NopCopy
- for (MCRegUnit SrcUnit : TRI.regunits(Src)) {
- auto SrcCopy = Copies.find(SrcUnit);
- if (SrcCopy != Copies.end() && SrcCopy->second.LastSeenUseInCopy) {
- // If SrcCopy defines multiple values, we only need
- // to erase the record for Def in DefRegs.
- for (auto itr = SrcCopy->second.DefRegs.begin();
- itr != SrcCopy->second.DefRegs.end(); itr++) {
- if (*itr == Def) {
- SrcCopy->second.DefRegs.erase(itr);
- // If DefReg becomes empty after removal, we can remove the
- // SrcCopy from the tracker's copy maps. We only remove those
- // entries solely record the Def is defined by Src. If an
- // entry also contains the definition record of other Def'
- // registers, it cannot be cleared.
- if (SrcCopy->second.DefRegs.empty() && !SrcCopy->second.MI) {
- Copies.erase(SrcCopy);
+ /// Clobber a single register, removing it from the tracker's copy maps.
+ void clobberRegister(MCRegister Reg, const TargetRegisterInfo &TRI,
+ const TargetInstrInfo &TII, bool UseCopyInstr) {
+ for (MCRegUnit Unit : TRI.regunits(Reg)) {
+ auto I = Copies.find(Unit);
+ if (I != Copies.end()) {
+ // When we clobber the source of a copy, we need to clobber everything
+ // it defined.
+ markRegsUnavailable(I->second.DefRegs, TRI);
+ // When we clobber the destination of a copy, we need to clobber the
+ // whole register it defined.
+ if (MachineInstr *MI = I->second.MI) {
+ std::optional<DestSourcePair> CopyOperands =
+ isCopyInstr(*MI, TII, UseCopyInstr);
+
+ MCRegister Def = CopyOperands->Destination->getReg().asMCReg();
+ MCRegister Src = CopyOperands->Source->getReg().asMCReg();
+
+ markRegsUnavailable(Def, TRI);
+
+ // Since we clobber the destination of a copy, the semantic of Src's
+ // "DefRegs" to contain Def is no longer effectual. We will also need
+ // to remove the record from the copy maps that indicates Src defined
+ // Def. Failing to do so might cause the target to miss some
+ // opportunities to further eliminate redundant copy instructions.
+ // Consider the following sequence during the
+ // ForwardCopyPropagateBlock procedure:
+ // L1: r0 = COPY r9 <- TrackMI
+ // L2: r0 = COPY r8 <- TrackMI (Remove r9 defined r0 from tracker)
+ // L3: use r0 <- Remove L2 from MaybeDeadCopies
+ // L4: early-clobber r9 <- Clobber r9 (L2 is still valid in tracker)
+ // L5: r0 = COPY r8 <- Remove NopCopy
+ for (MCRegUnit SrcUnit : TRI.regunits(Src)) {
+ auto SrcCopy = Copies.find(SrcUnit);
+ if (SrcCopy != Copies.end() && SrcCopy->second.LastSeenUseInCopy) {
+ // If SrcCopy defines multiple values, we only need
+ // to erase the record for Def in DefRegs.
+ for (auto itr = SrcCopy->second.DefRegs.begin();
+ itr != SrcCopy->second.DefRegs.end(); itr++) {
+ if (*itr == Def) {
+ SrcCopy->second.DefRegs.erase(itr);
+ // If DefReg becomes empty after removal, we can remove the
+ // SrcCopy from the tracker's copy maps. We only remove those
+ // entries solely record the Def is defined by Src. If an
+ // entry also contains the definition record of other Def'
+ // registers, it cannot be cleared.
+ if (SrcCopy->second.DefRegs.empty() && !SrcCopy->second.MI) {
+ Copies.erase(SrcCopy);
+ }
+ break;
}
- break;
}
}
}
}
+ // Now we can erase the copy.
+ Copies.erase(I);
}
- // Now we can erase the copy.
- Copies.erase(I);
}
}
- /// Clobber a single register, removing it from the tracker's copy maps.
- void clobberRegister(MCRegister Reg, const TargetRegisterInfo &TRI,
- const TargetInstrInfo &TII, bool UseCopyInstr) {
- for (MCRegUnit Unit : TRI.regunits(Reg)) {
- clobberRegUnit(Unit, TRI, TII, UseCopyInstr);
- }
- }
-
- /// Clobber all registers which are not preserved by RegMask, removing them
- /// from the tracker's copy maps.
- void clobberRegistersExceptMask(const MachineOperand *RegMask,
- const TargetRegisterInfo &TRI,
- const TargetInstrInfo &TII,
- bool UseCopyInstr) {
- BitVector SafeRegUnits(TRI.getNumRegUnits());
-
- for (unsigned SafeReg = 0, E = TRI.getNumRegs(); SafeReg < E; ++SafeReg)
- if (!RegMask->clobbersPhysReg(SafeReg))
- for (auto SafeUnit : TRI.regunits(SafeReg))
- SafeRegUnits.set(SafeUnit);
-
- for (unsigned Unit = 0, E = TRI.getNumRegUnits(); Unit < E; ++Unit)
- if (!SafeRegUnits.test(Unit))
- clobberRegUnit(Unit, TRI, TII, UseCopyInstr);
- }
-
/// Track copy's src users, and return false if that can't be done.
/// We can only track if we have a COPY instruction which source is
/// the same as the Reg.
@@ -984,10 +960,6 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
// a large set of registers. Treat clobbered registers the same way as
// defined registers.
if (RegMask) {
- // Invalidate all entries in the copy map which are not preserved by this
- // register mask.
- Tracker.clobberRegistersExceptMask(RegMask, *TRI, *TII, UseCopyInstr);
-
// Erase any MaybeDeadCopies whose destination register is clobbered.
for (SmallSetVector<MachineInstr *, 8>::iterator DI =
MaybeDeadCopies.begin();
@@ -1006,6 +978,10 @@ void MachineCopyPropagation::ForwardCopyPropagateBlock(MachineBasicBlock &MBB) {
LLVM_DEBUG(dbgs() << "MCP: Removing copy due to regmask clobbering: ";
MaybeDead->dump());
+ // Make sure we invalidate any entries in the copy maps before erasing
+ // the instruction.
+ Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
+
// erase() will return the next valid iterator pointing to the next
// element after the erased one.
DI = MaybeDeadCopies.erase(DI);
diff --git a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir
index e7865569c75bd0..5b379c2bd56292 100644
--- a/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir
+++ b/llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir
@@ -1,16 +1,5 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
-# RUN: llc -o - %s --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos | FileCheck %s
-
---- |
- declare void @foo()
-
- define void @test() {
- unreachable
- }
- define void @test2() {
- unreachable
- }
-...
+# RUN: llc -o - %s --run-pass=machine-cp -mcp-use-is-copy-instr -mtriple=arm64-apple-macos --verify-machineinstrs | FileCheck %s
---
name: test
@@ -41,22 +30,3 @@ body: |
RET undef $lr, implicit $x0
...
----
-name: test2
-tracksRegLiveness: true
-body: |
- bb.0:
- liveins: $q14, $d29, $x0, $x1
- ; CHECK-LABEL: name: test2
- ; CHECK: liveins: $q14, $d29, $x0, $x1
- ; CHECK-NEXT: {{ $}}
- ; CHECK-NEXT: renamable $d8 = COPY killed renamable $d29
- ; CHECK-NEXT: BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
- ; CHECK-NEXT: renamable $b0 = SMAXVv8i8v killed renamable $d8, implicit-def $q0
- ; CHECK-NEXT: RET_ReallyLR implicit $b0
- renamable $q8 = COPY renamable $q14
- renamable $d8 = COPY killed renamable $d29
- BL @foo, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
- renamable $b0 = SMAXVv8i8v killed renamable $d8, implicit-def $q0
- RET_ReallyLR implicit $b0
-...
More information about the llvm-commits
mailing list