caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "[MachineCP] Correctly handle register masks and sub-registers (#122472)"

Browse Source 
This reverts commit e2a071ece5.

This causes a large compile-time regression.
This commit is contained in:
Nikita Popov
2025-01-13 14:33:26 +01:00
parent ed80d0c2ae
commit d6f7f2a5fa
2 changed files with 59 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

140
llvm/lib/CodeGen/MachineCopyPropagation.cpp
View File

@@ -164,91 +164,67 @@ public:
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);
}
break;
}
}
}
}
}
// 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);
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;
}
}
}
}
}
// Now we can erase the copy.
Copies.erase(I);
}
}
}
/// 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);

32
llvm/test/CodeGen/AArch64/machine-cp-sub-reg.mir
View File

@@ -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
...