[llvm] r344942 - Reapply "[MachineCopyPropagation] Reimplement CopyTracker in terms of register units"
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 22 12:51:31 PDT 2018
Author: bogner
Date: Mon Oct 22 12:51:31 2018
New Revision: 344942
URL: http://llvm.org/viewvc/llvm-project?rev=344942&view=rev
Log:
Reapply "[MachineCopyPropagation] Reimplement CopyTracker in terms of register units"
Recommits r342942, which was reverted in r343189, with a fix for an
issue where we would propagate unsafely if we defined only the upper
part of a register.
Original message:
Change the copy tracker to keep a single map of register units
instead of 3 maps of registers. This gives a very significant
compile time performance improvement to the pass. I measured a
30-40% decrease in time spent in MCP on x86 and AArch64 and much
more significant improvements on out of tree targets with more
registers.
Added:
llvm/trunk/test/CodeGen/AArch64/machine-cp-clobbers.mir
llvm/trunk/test/CodeGen/Hexagon/machine-cp-clobbers.mir
Modified:
llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp
Modified: llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp?rev=344942&r1=344941&r2=344942&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineCopyPropagation.cpp Mon Oct 22 12:51:31 2018
@@ -75,98 +75,109 @@ DEBUG_COUNTER(FwdCounter, "machine-cp-fw
namespace {
class CopyTracker {
- using RegList = SmallVector<unsigned, 4>;
- using SourceMap = DenseMap<unsigned, RegList>;
- using Reg2MIMap = DenseMap<unsigned, MachineInstr *>;
+ struct CopyInfo {
+ MachineInstr *MI;
+ SmallVector<unsigned, 4> DefRegs;
+ bool Avail;
+ };
- /// Def -> available copies map.
- Reg2MIMap AvailCopyMap;
-
- /// Def -> copies map.
- Reg2MIMap CopyMap;
-
- /// Src -> Def map
- SourceMap SrcMap;
+ DenseMap<unsigned, CopyInfo> Copies;
public:
/// Mark all of the given registers and their subregisters as unavailable for
/// copying.
- void markRegsUnavailable(const RegList &Regs, const TargetRegisterInfo &TRI) {
+ void markRegsUnavailable(ArrayRef<unsigned> Regs,
+ const TargetRegisterInfo &TRI) {
for (unsigned Reg : Regs) {
// Source of copy is no longer available for propagation.
- for (MCSubRegIterator SR(Reg, &TRI, true); SR.isValid(); ++SR)
- AvailCopyMap.erase(*SR);
+ for (MCRegUnitIterator RUI(Reg, &TRI); RUI.isValid(); ++RUI) {
+ auto CI = Copies.find(*RUI);
+ if (CI != Copies.end())
+ CI->second.Avail = false;
+ }
}
}
/// Clobber a single register, removing it from the tracker's copy maps.
void clobberRegister(unsigned Reg, const TargetRegisterInfo &TRI) {
- for (MCRegAliasIterator AI(Reg, &TRI, true); AI.isValid(); ++AI) {
- CopyMap.erase(*AI);
- AvailCopyMap.erase(*AI);
-
- SourceMap::iterator SI = SrcMap.find(*AI);
- if (SI != SrcMap.end()) {
- markRegsUnavailable(SI->second, TRI);
- SrcMap.erase(SI);
+ for (MCRegUnitIterator RUI(Reg, &TRI); RUI.isValid(); ++RUI) {
+ auto I = Copies.find(*RUI);
+ 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)
+ markRegsUnavailable({MI->getOperand(0).getReg()}, TRI);
+ // Now we can erase the copy.
+ Copies.erase(I);
}
}
}
/// Add this copy's registers into the tracker's copy maps.
- void trackCopy(MachineInstr *Copy, const TargetRegisterInfo &TRI) {
- assert(Copy->isCopy() && "Tracking non-copy?");
+ void trackCopy(MachineInstr *MI, const TargetRegisterInfo &TRI) {
+ assert(MI->isCopy() && "Tracking non-copy?");
- unsigned Def = Copy->getOperand(0).getReg();
- unsigned Src = Copy->getOperand(1).getReg();
+ unsigned Def = MI->getOperand(0).getReg();
+ unsigned Src = MI->getOperand(1).getReg();
// Remember Def is defined by the copy.
- for (MCSubRegIterator SR(Def, &TRI, /*IncludeSelf=*/true); SR.isValid();
- ++SR) {
- CopyMap[*SR] = Copy;
- AvailCopyMap[*SR] = Copy;
- }
+ for (MCRegUnitIterator RUI(Def, &TRI); RUI.isValid(); ++RUI)
+ Copies[*RUI] = {MI, {}, true};
// Remember source that's copied to Def. Once it's clobbered, then
// it's no longer available for copy propagation.
- RegList &DestList = SrcMap[Src];
- if (!is_contained(DestList, Def))
- DestList.push_back(Def);
+ for (MCRegUnitIterator RUI(Src, &TRI); RUI.isValid(); ++RUI) {
+ auto I = Copies.insert({*RUI, {nullptr, {}, false}});
+ auto &Copy = I.first->second;
+ if (!is_contained(Copy.DefRegs, Def))
+ Copy.DefRegs.push_back(Def);
+ }
+ }
+
+ bool hasAnyCopies() {
+ return !Copies.empty();
}
- bool hasAvailableCopies() { return !AvailCopyMap.empty(); }
+ MachineInstr *findCopyForUnit(unsigned RegUnit, const TargetRegisterInfo &TRI,
+ bool MustBeAvailable = false) {
+ auto CI = Copies.find(RegUnit);
+ if (CI == Copies.end())
+ return nullptr;
+ if (MustBeAvailable && !CI->second.Avail)
+ return nullptr;
+ return CI->second.MI;
+ }
- MachineInstr *findAvailCopy(MachineInstr &DestCopy, unsigned Reg) {
- auto CI = AvailCopyMap.find(Reg);
- if (CI == AvailCopyMap.end())
+ MachineInstr *findAvailCopy(MachineInstr &DestCopy, unsigned Reg,
+ const TargetRegisterInfo &TRI) {
+ // We check the first RegUnit here, since we'll only be interested in the
+ // copy if it copies the entire register anyway.
+ MCRegUnitIterator RUI(Reg, &TRI);
+ MachineInstr *AvailCopy =
+ findCopyForUnit(*RUI, TRI, /*MustBeAvailable=*/true);
+ if (!AvailCopy ||
+ !TRI.isSubRegisterEq(AvailCopy->getOperand(0).getReg(), Reg))
return nullptr;
- MachineInstr &AvailCopy = *CI->second;
// Check that the available copy isn't clobbered by any regmasks between
// itself and the destination.
- unsigned AvailSrc = AvailCopy.getOperand(1).getReg();
- unsigned AvailDef = AvailCopy.getOperand(0).getReg();
+ unsigned AvailSrc = AvailCopy->getOperand(1).getReg();
+ unsigned AvailDef = AvailCopy->getOperand(0).getReg();
for (const MachineInstr &MI :
- make_range(AvailCopy.getIterator(), DestCopy.getIterator()))
+ make_range(AvailCopy->getIterator(), DestCopy.getIterator()))
for (const MachineOperand &MO : MI.operands())
if (MO.isRegMask())
if (MO.clobbersPhysReg(AvailSrc) || MO.clobbersPhysReg(AvailDef))
return nullptr;
- return &AvailCopy;
- }
-
- MachineInstr *findCopy(unsigned Reg) {
- auto CI = CopyMap.find(Reg);
- if (CI != CopyMap.end())
- return CI->second;
- return nullptr;
+ return AvailCopy;
}
void clear() {
- AvailCopyMap.clear();
- CopyMap.clear();
- SrcMap.clear();
+ Copies.clear();
}
};
@@ -224,8 +235,8 @@ INITIALIZE_PASS(MachineCopyPropagation,
void MachineCopyPropagation::ReadRegister(unsigned Reg) {
// If 'Reg' is defined by a copy, the copy is no longer a candidate
// for elimination.
- for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
- if (MachineInstr *Copy = Tracker.findCopy(*AI)) {
+ for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI) {
+ if (MachineInstr *Copy = Tracker.findCopyForUnit(*RUI, *TRI)) {
LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump());
MaybeDeadCopies.remove(Copy);
}
@@ -263,7 +274,7 @@ bool MachineCopyPropagation::eraseIfRedu
return false;
// Search for an existing copy.
- MachineInstr *PrevCopy = Tracker.findAvailCopy(Copy, Def);
+ MachineInstr *PrevCopy = Tracker.findAvailCopy(Copy, Def, *TRI);
if (!PrevCopy)
return false;
@@ -357,7 +368,7 @@ bool MachineCopyPropagation::hasImplicit
/// Look for available copies whose destination register is used by \p MI and
/// replace the use in \p MI with the copy's source register.
void MachineCopyPropagation::forwardUses(MachineInstr &MI) {
- if (!Tracker.hasAvailableCopies())
+ if (!Tracker.hasAnyCopies())
return;
// Look for non-tied explicit vreg uses that have an active COPY
@@ -384,7 +395,7 @@ void MachineCopyPropagation::forwardUses
if (!MOUse.isRenamable())
continue;
- MachineInstr *Copy = Tracker.findAvailCopy(MI, MOUse.getReg());
+ MachineInstr *Copy = Tracker.findAvailCopy(MI, MOUse.getReg(), *TRI);
if (!Copy)
continue;
Added: llvm/trunk/test/CodeGen/AArch64/machine-cp-clobbers.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/machine-cp-clobbers.mir?rev=344942&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/machine-cp-clobbers.mir (added)
+++ llvm/trunk/test/CodeGen/AArch64/machine-cp-clobbers.mir Mon Oct 22 12:51:31 2018
@@ -0,0 +1,51 @@
+# RUN: llc -march=aarch64 -o - %s -run-pass=machine-cp | FileCheck %s
+
+---
+name: dont_propagate_past_lower_subreg_kill
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: dont_propagate_past_lower_subreg_kill
+ ; CHECK: HINT 0, implicit-def $q0
+ ; CHECK: HINT 0, implicit-def $d1
+ ; CHECK: HINT 0, implicit killed $d1
+ ; CHECK: $q1 = COPY killed $q0
+ ; CHECK: $q2 = COPY $q1
+ ; CHECK: HINT 0, implicit $q2
+ HINT 0, implicit-def $q0
+ $q1 = COPY killed $q0
+ $q0 = COPY killed $q1
+
+ HINT 0, implicit-def $d1
+ HINT 0, implicit killed $d1
+
+ $q1 = COPY killed $q0
+ $q2 = COPY $q1
+ HINT 0, implicit $q2
+
+...
+
+---
+name: dont_propagate_past_upper_subreg_kill
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: dont_propagate_past_upper_subreg_kill
+ ; CHECK: HINT 0, implicit-def $z0
+ ; CHECK: HINT 0, implicit-def $z1_hi
+ ; CHECK: HINT 0, implicit killed $z1_hi
+ ; CHECK: $z1 = COPY killed $z0
+ ; CHECK: $z2 = COPY $z1
+ ; CHECK: HINT 0, implicit $z2
+ HINT 0, implicit-def $z0
+ $z1 = COPY killed $z0
+ $z0 = COPY killed $z1
+
+ HINT 0, implicit-def $z1_hi
+ HINT 0, implicit killed $z1_hi
+
+ $z1 = COPY killed $z0
+ $z2 = COPY $z1
+ HINT 0, implicit $z2
+
+...
Added: llvm/trunk/test/CodeGen/Hexagon/machine-cp-clobbers.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/machine-cp-clobbers.mir?rev=344942&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/machine-cp-clobbers.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/machine-cp-clobbers.mir Mon Oct 22 12:51:31 2018
@@ -0,0 +1,51 @@
+# RUN: llc -march=hexagon -o - %s -run-pass=machine-cp | FileCheck %s
+
+---
+name: dont_propagate_past_lower_subreg_kill
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: dont_propagate_past_lower_subreg_kill
+ ; CHECK: A2_nop implicit-def $d0
+ ; CHECK: A2_nop implicit-def $r2
+ ; CHECK: A2_nop implicit killed $r2
+ ; CHECK: $d1 = COPY killed $d0
+ ; CHECK: $d2 = COPY $d1
+ ; CHECK: A2_nop implicit $d2
+ A2_nop implicit-def $d0
+ $d1 = COPY killed $d0
+ $d0 = COPY killed $d1
+
+ A2_nop implicit-def $r2
+ A2_nop implicit killed $r2
+
+ $d1 = COPY killed $d0
+ $d2 = COPY $d1
+ A2_nop implicit $d2
+
+...
+
+---
+name: dont_propagate_past_upper_subreg_kill
+tracksRegLiveness: true
+body: |
+ bb.0:
+ ; CHECK-LABEL: name: dont_propagate_past_upper_subreg_kill
+ ; CHECK: A2_nop implicit-def $d0
+ ; CHECK: A2_nop implicit-def $r3
+ ; CHECK: A2_nop implicit killed $r3
+ ; CHECK: $d1 = COPY killed $d0
+ ; CHECK: $d2 = COPY $d1
+ ; CHECK: A2_nop implicit $d2
+ A2_nop implicit-def $d0
+ $d1 = COPY killed $d0
+ $d0 = COPY killed $d1
+
+ A2_nop implicit-def $r3
+ A2_nop implicit killed $r3
+
+ $d1 = COPY killed $d0
+ $d2 = COPY $d1
+ A2_nop implicit $d2
+
+...
More information about the llvm-commits
mailing list