Skip to content

Commit

Permalink
MachineCopyPropagation: Keep scanning through instructions with regmasks
Browse files Browse the repository at this point in the history 
This also simplifies the code by removing the overly conservative
NoInterveningSideEffect() function. This function checked:
- That the two copies belong to the same block: We only process one
  block at a time and clear our maps in between it is impossible to find a
  copy from a different block.
- There is no terminator between the two copy instructions: This is not
  allowed anyway (the MachineVerifier would complain)
- Does not have instructions with hasUnmodeledSideEffects() or isCall()
  set: Even for those instructuction we must have all clobbers/defs of
  registers explicit as an operand. If the register is explicitely
  clobbered we would never come to the point of checking for
  NoInterveningSideEffect() anyway.

(I also checked this with a temporary build of the test-suite with all
 potentially failing conditions in NoInterveningSideEffect() turned into
 asserts)

Differential Revision: http://reviews.llvm.org/D17474

llvm-svn: 261965
  • Loading branch information
@MatzeB
MatzeB committed Feb 26, 2016
1 parent e3fda8a commit e39ff70
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 59 deletions.
110 changes: 55 additions & 55 deletions llvm/lib/CodeGen/MachineCopyPropagation.cpp
View file
Expand Up @@ -32,6 +32,10 @@ using namespace llvm;
STATISTIC(NumDeletes, "Number of dead copies deleted");

namespace {
typedef SmallVector<unsigned, 4> RegList;
typedef DenseMap<unsigned, RegList> SourceMap;
typedef DenseMap<unsigned, MachineInstr*> Reg2MIMap;

class MachineCopyPropagation : public MachineFunctionPass {
const TargetRegisterInfo *TRI;
const TargetInstrInfo *TII;
Expand All @@ -46,11 +50,7 @@ namespace {
bool runOnMachineFunction(MachineFunction &MF) override;

private:
typedef SmallVector<unsigned, 4> DestList;
typedef DenseMap<unsigned, DestList> SourceMap;
typedef DenseMap<unsigned, MachineInstr*> Reg2MIMap;

void SourceNoLongerAvailable(unsigned Reg);
void ClobberRegister(unsigned Reg);
void CopyPropagateBlock(MachineBasicBlock &MBB);

/// Candidates for deletion.
Expand All @@ -70,33 +70,43 @@ char &llvm::MachineCopyPropagationID = MachineCopyPropagation::ID;
INITIALIZE_PASS(MachineCopyPropagation, "machine-cp",
"Machine Copy Propagation Pass", false, false)

void MachineCopyPropagation::SourceNoLongerAvailable(unsigned Reg) {
for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
SourceMap::iterator SI = SrcMap.find(*AI);
if (SI != SrcMap.end()) {
const DestList& Defs = SI->second;
for (unsigned MappedDef : Defs) {
// Source of copy is no longer available for propagation.
for (MCSubRegIterator SR(MappedDef, TRI, true); SR.isValid(); ++SR)
AvailCopyMap.erase(*SR);
}
}
/// Remove any entry in \p Map where the register is a subregister or equal to
/// a register contained in \p Regs.
static void removeRegsFromMap(Reg2MIMap &Map, const RegList &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)
Map.erase(*SR);
}
}

static bool NoInterveningSideEffect(const MachineInstr *CopyMI,
const MachineInstr *MI) {
const MachineBasicBlock *MBB = CopyMI->getParent();
if (MI->getParent() != MBB)
return false;
/// Remove any entry in \p Map that is marked clobbered in \p RegMask.
/// The map will typically have a lot fewer entries than the regmask clobbers,
/// so this is more efficient than iterating the clobbered registers and calling
/// ClobberRegister() on them.
static void removeClobberedRegsFromMap(Reg2MIMap &Map,
const MachineOperand &RegMask) {
for (Reg2MIMap::iterator I = Map.begin(), E = Map.end(), Next; I != E;
I = Next) {
Next = std::next(I);
unsigned Reg = I->first;
if (RegMask.clobbersPhysReg(Reg))
Map.erase(I);
}
}

for (MachineBasicBlock::const_iterator I = std::next(CopyMI->getIterator()),
E = MBB->end(), E2 = MI->getIterator(); I != E && I != E2; ++I) {
if (I->hasUnmodeledSideEffects() || I->isCall() ||
I->isTerminator())
return false;
void MachineCopyPropagation::ClobberRegister(unsigned Reg) {
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()) {
removeRegsFromMap(AvailCopyMap, SI->second, *TRI);
SrcMap.erase(SI);
}
}
return true;
}

/// isNopCopy - Return true if the specified copy is really a nop. That is
Expand Down Expand Up @@ -142,9 +152,7 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
DenseMap<unsigned, MachineInstr*>::iterator CI = AvailCopyMap.find(Src);
if (CI != AvailCopyMap.end()) {
MachineInstr *CopyMI = CI->second;
if (!MRI->isReserved(Def) &&
(!MRI->isReserved(Src) || NoInterveningSideEffect(CopyMI, MI)) &&
isNopCopy(CopyMI, Def, Src, TRI)) {
if (!MRI->isReserved(Def) && isNopCopy(CopyMI, Def, Src, TRI)) {
// The two copies cancel out and the source of the first copy
// hasn't been overridden, eliminate the second one. e.g.
// %ECX<def> = COPY %EAX<kill>
Expand Down Expand Up @@ -197,14 +205,9 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
// %xmm2<def> = copy %xmm0
// ...
// %xmm2<def> = copy %xmm9
SourceNoLongerAvailable(Def);
ClobberRegister(Def);

// Remember Def is defined by the copy.
// ... Make sure to clear the def maps of aliases first.
for (MCRegAliasIterator AI(Def, TRI, false); AI.isValid(); ++AI) {
CopyMap.erase(*AI);
AvailCopyMap.erase(*AI);
}
for (MCSubRegIterator SR(Def, TRI, /*IncludeSelf=*/true); SR.isValid();
++SR) {
CopyMap[*SR] = MI;
Expand All @@ -213,7 +216,7 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {

// Remember source that's copied to Def. Once it's clobbered, then
// it's no longer available for copy propagation.
SmallVectorImpl<unsigned> &DestList = SrcMap[Src];
RegList &DestList = SrcMap[Src];
if (std::find(DestList.begin(), DestList.end(), Def) == DestList.end())
DestList.push_back(Def);

Expand Down Expand Up @@ -260,9 +263,8 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
}

// The instruction has a register mask operand which means that it clobbers
// a large set of registers. It is possible to use the register mask to
// prune the available copies, but treat it like a basic block boundary for
// now.
// a large set of registers. Treat clobbered registers the same way as
// defined registers.
if (RegMask) {
// Erase any MaybeDeadCopies whose destination register is clobbered.
for (MachineInstr *MaybeDead : MaybeDeadCopies) {
Expand All @@ -276,25 +278,23 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) {
Changed = true;
++NumDeletes;
}

// Clear all data structures as if we were beginning a new basic block.
MaybeDeadCopies.clear();
AvailCopyMap.clear();
CopyMap.clear();
SrcMap.clear();
continue;
}

for (unsigned Reg : Defs) {
// No longer defined by a copy.
for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
CopyMap.erase(*AI);
AvailCopyMap.erase(*AI);
removeClobberedRegsFromMap(AvailCopyMap, *RegMask);
removeClobberedRegsFromMap(CopyMap, *RegMask);
for (SourceMap::iterator I = SrcMap.begin(), E = SrcMap.end(), Next;
I != E; I = Next) {
Next = std::next(I);
if (RegMask->clobbersPhysReg(I->first)) {
removeRegsFromMap(AvailCopyMap, I->second, *TRI);
SrcMap.erase(I);
}
}
}

// If 'Reg' is previously source of a copy, it is no longer available for
// copy propagation.
SourceNoLongerAvailable(Reg);
// Any previous copy definition or reading the Defs is no longer available.
for (unsigned Reg : Defs) {
ClobberRegister(Reg);
}
}

Expand Down
63 changes: 59 additions & 4 deletions llvm/test/CodeGen/X86/machine-copy-prop.mir
View file
@@ -1,13 +1,16 @@
# RUN: llc -march=x86 -run-pass machine-cp -verify-machineinstrs -o /dev/null %s 2>&1 | FileCheck %s
# RUN: llc -march=x86 -run-pass machine-cp -o /dev/null %s 2>&1 | FileCheck %s

--- |
declare void @foo()
define void @copyprop_remove_kill0() { ret void }
define void @copyprop_remove_kill1() { ret void }
define void @copyprop_remove_kill2() { ret void }
define void @copyprop0() { ret void }
define void @nocopyprop0() { ret void }
define void @nocopyprop1() { ret void }
...
---
# The second copy is redundand and will be removed, check that we also remove
# The second copy is redundant and will be removed, check that we also remove
# the kill flag of intermediate instructions.
# CHECK-LABEL: name: copyprop_remove_kill0
# CHECK: bb.0:
Expand All @@ -24,7 +27,7 @@ body: |
NOOP implicit %rax, implicit %rdi
...
---
# The second copy is redundand and will be removed, check that we also remove
# The second copy is redundant and will be removed, check that we also remove
# the kill flag of intermediate instructions.
# CHECK-LABEL: name: copyprop_remove_kill1
# CHECK: bb.0:
Expand All @@ -41,7 +44,7 @@ body: |
NOOP implicit %rax, implicit %rdi
...
---
# The second copy is redundand and will be removed, check that we also remove
# The second copy is redundant and will be removed, check that we also remove
# the kill flag of intermediate instructions.
# CHECK-LABEL: name: copyprop_remove_kill2
# CHECK: bb.0:
Expand All @@ -57,3 +60,55 @@ body: |
%di = COPY %ax
NOOP implicit %rax, implicit %rdi
...
---
# The second copy is redundant; the call preserves the source and dest register.
# CHECK-LABEL: name: copyprop0
# CHECK: bb.0:
# CHECK-NEXT: %rax = COPY %rdi
# CHECK-NEXT: CALL64pcrel32 @foo, csr_64_rt_mostregs
# CHECK-NEXT: NOOP implicit %edi
# CHECK-NOT: COPY
# CHECK-NEXT: NOOP implicit %rax, implicit %rdi
name: copyprop0
body: |
bb.0:
%rax = COPY %rdi
CALL64pcrel32 @foo, csr_64_rt_mostregs
NOOP implicit killed %edi
%rdi = COPY %rax
NOOP implicit %rax, implicit %rdi
...
---
# The second copy is not redundant if the source register (%rax) is clobbered
# even if the dest (%rbp) is not.
# CHECK-LABEL: name: nocopyprop0
# CHECK: bb.0:
# CHECK-NEXT: %rax = COPY %rbp
# CHECK-NEXT: CALL64pcrel32 @foo, csr_64, implicit %rax, implicit %rbp
# CHECK-NEXT: %rbp = COPY %rax
# CHECK-NEXT: NOOP implicit %rax, implicit %rbp
name: nocopyprop0
body: |
bb.0:
%rax = COPY %rbp
CALL64pcrel32 @foo, csr_64, implicit %rax, implicit %rbp
%rbp = COPY %rax
NOOP implicit %rax, implicit %rbp
...
---
# The second copy is not redundant if the dest register (%rax) is clobbered
# even if the source (%rbp) is not.
# CHECK-LABEL: name: nocopyprop1
# CHECK: bb.0:
# CHECK-NEXT: %rbp = COPY %rax
# CHECK-NEXT: CALL64pcrel32 @foo, csr_64, implicit %rax, implicit %rbp
# CHECK-NEXT: %rax = COPY %rbp
# CHECK-NEXT: NOOP implicit %rax, implicit %rbp
name: nocopyprop1
body: |
bb.0:
%rbp = COPY %rax
CALL64pcrel32 @foo, csr_64, implicit %rax, implicit %rbp
%rax = COPY %rbp
NOOP implicit %rax, implicit %rbp
...

0 comments on commit e39ff70

Please sign in to comment.