Index: lib/CodeGen/MachineCopyPropagation.cpp =================================================================== --- lib/CodeGen/MachineCopyPropagation.cpp +++ lib/CodeGen/MachineCopyPropagation.cpp @@ -32,6 +32,10 @@ STATISTIC(NumDeletes, "Number of dead copies deleted"); namespace { + typedef SmallVector RegList; + typedef DenseMap SourceMap; + typedef DenseMap Reg2MIMap; + class MachineCopyPropagation : public MachineFunctionPass { const TargetRegisterInfo *TRI; const TargetInstrInfo *TII; @@ -46,11 +50,7 @@ bool runOnMachineFunction(MachineFunction &MF) override; private: - typedef SmallVector DestList; - typedef DenseMap SourceMap; - typedef DenseMap Reg2MIMap; - - void SourceNoLongerAvailable(unsigned Reg); + void ClobberRegister(unsigned Reg); void CopyPropagateBlock(MachineBasicBlock &MBB); /// Candidates for deletion. @@ -70,33 +70,43 @@ 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 @@ -142,9 +152,7 @@ DenseMap::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 = COPY %EAX @@ -197,14 +205,9 @@ // %xmm2 = copy %xmm0 // ... // %xmm2 = 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; @@ -260,9 +263,8 @@ } // 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) { @@ -276,25 +278,23 @@ 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); } } Index: test/CodeGen/X86/machine-copy-prop.mir =================================================================== --- test/CodeGen/X86/machine-copy-prop.mir +++ test/CodeGen/X86/machine-copy-prop.mir @@ -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: @@ -24,7 +27,7 @@ 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: @@ -41,7 +44,7 @@ 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: @@ -57,3 +60,55 @@ %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 +...