From 86c40db49d89699d573347f093c3ea6a8a08eb61 Mon Sep 17 00:00:00 2001 From: Jonas Paulsson Date: Tue, 5 Dec 2017 10:52:24 +0000 Subject: [PATCH] [Regalloc] Generate and store multiple regalloc hints. MachineRegisterInfo used to allow just one regalloc hint per virtual register. This patch extends this to a vector of regalloc hints, which is filled in by common code with sorted copy hints. Such hints will make for more ID copies that can be removed. NB! This improvement is currently (and hopefully temporarily) *disabled* by default, except for SystemZ. The only reason for this is the big impact this has on tests, which has unfortunately proven unmanageable. It was a long while since all the tests were updated and just waiting for review (which didn't happen), but now targets have to enable this themselves instead. Several targets could get a head-start by downloading the tests updates from the Phabricator review. Thanks to those who helped, and sorry you now have to do this step yourselves. This should be an improvement generally for any target! The target may still create its own hint, in which case this has highest priority and is stored first in the vector. If it has target-type, it will not be recomputed, as per the previous behaviour. The temporary hook enableMultipleCopyHints() will be removed as soon as all targets return true. Review: Quentin Colombet, Ulrich Weigand. https://reviews.llvm.org/D38128 llvm-svn: 319754 --- .../llvm/CodeGen/MachineRegisterInfo.h | 59 ++++++++--- .../include/llvm/CodeGen/TargetRegisterInfo.h | 16 ++- llvm/lib/CodeGen/CalcSpillWeights.cpp | 97 +++++++++++++------ llvm/lib/CodeGen/TargetRegisterInfo.cpp | 47 +++++---- llvm/lib/Target/SystemZ/SystemZRegisterInfo.h | 2 + llvm/test/CodeGen/SystemZ/call-03.ll | 5 +- llvm/test/CodeGen/SystemZ/swift-return.ll | 3 +- llvm/test/CodeGen/SystemZ/swifterror.ll | 23 +++-- 8 files changed, 163 insertions(+), 89 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h index be9b89eb77ef..3be94f802170 100644 --- a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h +++ b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h @@ -84,14 +84,15 @@ private: /// all registers that were disabled are removed from the list. SmallVector UpdatedCSRs; - /// RegAllocHints - This vector records register allocation hints for virtual - /// registers. For each virtual register, it keeps a register and hint type - /// pair making up the allocation hint. Hint type is target specific except - /// for the value 0 which means the second value of the pair is the preferred - /// register for allocation. For example, if the hint is <0, 1024>, it means - /// the allocator should prefer the physical register allocated to the virtual - /// register of the hint. - IndexedMap, VirtReg2IndexFunctor> RegAllocHints; + /// RegAllocHints - This vector records register allocation hints for + /// virtual registers. For each virtual register, it keeps a pair of hint + /// type and hints vector making up the allocation hints. Only the first + /// hint may be target specific, and in that case this is reflected by the + /// first member of the pair being non-zero. If the hinted register is + /// virtual, it means the allocator should prefer the physical register + /// allocated to it if any. + IndexedMap>, + VirtReg2IndexFunctor> RegAllocHints; /// PhysRegUseDefLists - This is an array of the head of the use/def list for /// physical registers. @@ -706,35 +707,61 @@ public: void clearVirtRegs(); /// setRegAllocationHint - Specify a register allocation hint for the - /// specified virtual register. + /// specified virtual register. This is typically used by target, and in case + /// of an earlier hint it will be overwritten. void setRegAllocationHint(unsigned VReg, unsigned Type, unsigned PrefReg) { assert(TargetRegisterInfo::isVirtualRegister(VReg)); RegAllocHints[VReg].first = Type; - RegAllocHints[VReg].second = PrefReg; + RegAllocHints[VReg].second.clear(); + RegAllocHints[VReg].second.push_back(PrefReg); } - /// Specify the preferred register allocation hint for the specified virtual - /// register. + /// addRegAllocationHint - Add a register allocation hint to the hints + /// vector for VReg. + void addRegAllocationHint(unsigned VReg, unsigned PrefReg) { + assert(TargetRegisterInfo::isVirtualRegister(VReg)); + RegAllocHints[VReg].second.push_back(PrefReg); + } + + /// Specify the preferred (target independent) register allocation hint for + /// the specified virtual register. void setSimpleHint(unsigned VReg, unsigned PrefReg) { setRegAllocationHint(VReg, /*Type=*/0, PrefReg); } + void clearSimpleHint(unsigned VReg) { + assert (RegAllocHints[VReg].first == 0 && + "Expected to clear a non-target hint!"); + RegAllocHints[VReg].second.clear(); + } + /// getRegAllocationHint - Return the register allocation hint for the - /// specified virtual register. + /// specified virtual register. If there are many hints, this returns the + /// one with the greatest weight. std::pair getRegAllocationHint(unsigned VReg) const { assert(TargetRegisterInfo::isVirtualRegister(VReg)); - return RegAllocHints[VReg]; + unsigned BestHint = (RegAllocHints[VReg].second.size() ? + RegAllocHints[VReg].second[0] : 0); + return std::pair(RegAllocHints[VReg].first, BestHint); } - /// getSimpleHint - Return the preferred register allocation hint, or 0 if a - /// standard simple hint (Type == 0) is not set. + /// getSimpleHint - same as getRegAllocationHint except it will only return + /// a target independent hint. unsigned getSimpleHint(unsigned VReg) const { assert(TargetRegisterInfo::isVirtualRegister(VReg)); std::pair Hint = getRegAllocationHint(VReg); return Hint.first ? 0 : Hint.second; } + /// getRegAllocationHints - Return a reference to the vector of all + /// register allocation hints for VReg. + const std::pair> + &getRegAllocationHints(unsigned VReg) const { + assert(TargetRegisterInfo::isVirtualRegister(VReg)); + return RegAllocHints[VReg]; + } + /// markUsesInDebugValueAsUndef - Mark every DBG_VALUE referencing the /// specified register as undefined which causes the DBG_VALUE to be /// deleted during LiveDebugVariables analysis. diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h index 92d38d51feef..cc612a42d756 100644 --- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h +++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h @@ -785,11 +785,10 @@ public: /// as returned from RegisterClassInfo::getOrder(). The hint registers must /// come from Order, and they must not be reserved. /// - /// The default implementation of this function can resolve - /// target-independent hints provided to MRI::setRegAllocationHint with - /// HintType == 0. Targets that override this function should defer to the - /// default implementation if they have no reason to change the allocation - /// order for VirtReg. There may be target-independent hints. + /// The default implementation of this function will only add target + /// independent register allocation hints. Targets that override this + /// function should typically call this default implementation as well and + /// expect to see generic copy hints added. virtual bool getRegAllocationHints(unsigned VirtReg, ArrayRef Order, SmallVectorImpl &Hints, @@ -808,6 +807,13 @@ public: // Do nothing. } + /// The creation of multiple copy hints have been implemented in + /// weightCalcHelper(), but since this affects so many tests for many + /// targets, this is temporarily disabled per default. THIS SHOULD BE + /// "GENERAL GOODNESS" and hopefully all targets will update their tests + /// and enable this soon. This hook should then be removed. + virtual bool enableMultipleCopyHints() const { return false; } + /// Allow the target to reverse allocation order of local live ranges. This /// will generally allocate shorter local live ranges first. For targets with /// many registers, this could reduce regalloc compile time by a large diff --git a/llvm/lib/CodeGen/CalcSpillWeights.cpp b/llvm/lib/CodeGen/CalcSpillWeights.cpp index 6a6ec461cf70..f52a068a4cd1 100644 --- a/llvm/lib/CodeGen/CalcSpillWeights.cpp +++ b/llvm/lib/CodeGen/CalcSpillWeights.cpp @@ -70,13 +70,24 @@ static unsigned copyHint(const MachineInstr *mi, unsigned reg, return sub == hsub ? hreg : 0; const TargetRegisterClass *rc = mri.getRegClass(reg); + if (!tri.enableMultipleCopyHints()) { + // Only allow physreg hints in rc. + if (sub == 0) + return rc->contains(hreg) ? hreg : 0; - // Only allow physreg hints in rc. - if (sub == 0) - return rc->contains(hreg) ? hreg : 0; + // reg:sub should match the physreg hreg. + return tri.getMatchingSuperReg(hreg, sub, rc); + } - // reg:sub should match the physreg hreg. - return tri.getMatchingSuperReg(hreg, sub, rc); + unsigned CopiedPReg = (hsub ? tri.getSubReg(hreg, hsub) : hreg); + if (rc->contains(CopiedPReg)) + return CopiedPReg; + + // Check if reg:sub matches so that a super register could be hinted. + if (sub) + return tri.getMatchingSuperReg(CopiedPReg, sub, rc); + + return 0; } // Check if all values in LI are rematerializable @@ -157,12 +168,7 @@ float VirtRegAuxInfo::weightCalcHelper(LiveInterval &li, SlotIndex *start, unsigned numInstr = 0; // Number of instructions using li SmallPtrSet visited; - // Find the best physreg hint and the best virtreg hint. - float bestPhys = 0, bestVirt = 0; - unsigned hintPhys = 0, hintVirt = 0; - - // Don't recompute a target specific hint. - bool noHint = mri.getRegAllocationHint(li.reg).first != 0; + std::pair TargetHint = mri.getRegAllocationHint(li.reg); // Don't recompute spill weight for an unspillable register. bool Spillable = li.isSpillable(); @@ -188,6 +194,36 @@ float VirtRegAuxInfo::weightCalcHelper(LiveInterval &li, SlotIndex *start, numInstr += 2; } + // CopyHint is a sortable hint derived from a COPY instruction. + struct CopyHint { + unsigned Reg; + float Weight; + bool IsPhys; + unsigned HintOrder; + CopyHint(unsigned R, float W, bool P, unsigned HR) : + Reg(R), Weight(W), IsPhys(P), HintOrder(HR) {} + bool operator<(const CopyHint &rhs) const { + // Always prefer any physreg hint. + if (IsPhys != rhs.IsPhys) + return (IsPhys && !rhs.IsPhys); + if (Weight != rhs.Weight) + return (Weight > rhs.Weight); + + // This is just a temporary way to achive NFC for targets that don't + // enable multiple copy hints. HintOrder should be removed when all + // targets return true in enableMultipleCopyHints(). + return (HintOrder < rhs.HintOrder); + +#if 0 // Should replace the HintOrder check, see above. + // (just for the purpose of maintaining the set) + return Reg < rhs.Reg; +#endif + } + }; + std::set CopyHints; + + // Temporary: see comment for HintOrder above. + unsigned CopyHintOrder = 0; for (MachineRegisterInfo::reg_instr_iterator I = mri.reg_instr_begin(li.reg), E = mri.reg_instr_end(); I != E; ) { @@ -227,7 +263,8 @@ float VirtRegAuxInfo::weightCalcHelper(LiveInterval &li, SlotIndex *start, } // Get allocation hints from copies. - if (noHint || !mi->isCopy()) + if (!mi->isCopy() || + (TargetHint.first != 0 && !tri.enableMultipleCopyHints())) continue; unsigned hint = copyHint(mi, li.reg, tri, mri); if (!hint) @@ -237,28 +274,30 @@ float VirtRegAuxInfo::weightCalcHelper(LiveInterval &li, SlotIndex *start, // // FIXME: we probably shouldn't use floats at all. volatile float hweight = Hint[hint] += weight; - if (TargetRegisterInfo::isPhysicalRegister(hint)) { - if (hweight > bestPhys && mri.isAllocatable(hint)) { - bestPhys = hweight; - hintPhys = hint; - } - } else { - if (hweight > bestVirt) { - bestVirt = hweight; - hintVirt = hint; - } - } + if (TargetRegisterInfo::isVirtualRegister(hint) || mri.isAllocatable(hint)) + CopyHints.insert(CopyHint(hint, hweight, tri.isPhysicalRegister(hint), + (tri.enableMultipleCopyHints() ? hint : CopyHintOrder++))); } Hint.clear(); - // Always prefer the physreg hint. - if (updateLI) { - if (unsigned hint = hintPhys ? hintPhys : hintVirt) { - mri.setRegAllocationHint(li.reg, 0, hint); - // Weakly boost the spill weight of hinted registers. - totalWeight *= 1.01F; + // Pass all the sorted copy hints to mri. + if (updateLI && CopyHints.size()) { + // Remove a generic hint if previously added by target. + if (TargetHint.first == 0 && TargetHint.second) + mri.clearSimpleHint(li.reg); + + for (auto &Hint : CopyHints) { + if (TargetHint.first != 0 && Hint.Reg == TargetHint.second) + // Don't add again the target-type hint. + continue; + mri.addRegAllocationHint(li.reg, Hint.Reg); + if (!tri.enableMultipleCopyHints()) + break; } + + // Weakly boost the spill weight of hinted registers. + totalWeight *= 1.01F; } // If the live interval was already unspillable, leave it that way. diff --git a/llvm/lib/CodeGen/TargetRegisterInfo.cpp b/llvm/lib/CodeGen/TargetRegisterInfo.cpp index f4e5583cbe7b..dfda313f2336 100644 --- a/llvm/lib/CodeGen/TargetRegisterInfo.cpp +++ b/llvm/lib/CodeGen/TargetRegisterInfo.cpp @@ -373,31 +373,36 @@ TargetRegisterInfo::getRegAllocationHints(unsigned VirtReg, const VirtRegMap *VRM, const LiveRegMatrix *Matrix) const { const MachineRegisterInfo &MRI = MF.getRegInfo(); - std::pair Hint = MRI.getRegAllocationHint(VirtReg); + const std::pair> &Hints_MRI = + MRI.getRegAllocationHints(VirtReg); - // Hints with HintType != 0 were set by target-dependent code. - // Such targets must provide their own implementation of - // TRI::getRegAllocationHints to interpret those hint types. - assert(Hint.first == 0 && "Target must implement TRI::getRegAllocationHints"); + // First hint may be a target hint. + bool Skip = (Hints_MRI.first != 0); + for (auto Reg : Hints_MRI.second) { + if (Skip) { + Skip = false; + continue; + } - // Target-independent hints are either a physical or a virtual register. - unsigned Phys = Hint.second; - if (VRM && isVirtualRegister(Phys)) - Phys = VRM->getPhys(Phys); + // Target-independent hints are either a physical or a virtual register. + unsigned Phys = Reg; + if (VRM && isVirtualRegister(Phys)) + Phys = VRM->getPhys(Phys); - // Check that Phys is a valid hint in VirtReg's register class. - if (!isPhysicalRegister(Phys)) - return false; - if (MRI.isReserved(Phys)) - return false; - // Check that Phys is in the allocation order. We shouldn't heed hints - // from VirtReg's register class if they aren't in the allocation order. The - // target probably has a reason for removing the register. - if (!is_contained(Order, Phys)) - return false; + // Check that Phys is a valid hint in VirtReg's register class. + if (!isPhysicalRegister(Phys)) + continue; + if (MRI.isReserved(Phys)) + continue; + // Check that Phys is in the allocation order. We shouldn't heed hints + // from VirtReg's register class if they aren't in the allocation order. The + // target probably has a reason for removing the register. + if (!is_contained(Order, Phys)) + continue; - // All clear, tell the register allocator to prefer this register. - Hints.push_back(Phys); + // All clear, tell the register allocator to prefer this register. + Hints.push_back(Phys); + } return false; } diff --git a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.h b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.h index 5f8f8ca9143d..8787a90b1e25 100644 --- a/llvm/lib/Target/SystemZ/SystemZRegisterInfo.h +++ b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.h @@ -51,6 +51,8 @@ public: const VirtRegMap *VRM, const LiveRegMatrix *Matrix) const override; + bool enableMultipleCopyHints() const override { return true; } + // Override TargetRegisterInfo.h. bool requiresRegisterScavenging(const MachineFunction &MF) const override { return true; diff --git a/llvm/test/CodeGen/SystemZ/call-03.ll b/llvm/test/CodeGen/SystemZ/call-03.ll index 4c2eed0f56d8..1f314eae58c8 100644 --- a/llvm/test/CodeGen/SystemZ/call-03.ll +++ b/llvm/test/CodeGen/SystemZ/call-03.ll @@ -62,16 +62,13 @@ define void @f4() { ; Check an indirect call. In this case the only acceptable choice for ; the target register is %r1. -; -; NOTE: the extra copy 'lgr %r1, %r0' is a coalescing failure. define void @f5(void(i32, i32, i32, i32) *%foo) { ; CHECK-LABEL: f5: -; CHECK: lgr %r0, %r2 +; CHECK: lgr %r1, %r2 ; CHECK-DAG: lhi %r2, 1 ; CHECK-DAG: lhi %r3, 2 ; CHECK-DAG: lhi %r4, 3 ; CHECK-DAG: lhi %r5, 4 -; CHECK: lgr %r1, %r0 ; CHECK: br %r1 tail call void %foo(i32 1, i32 2, i32 3, i32 4) ret void diff --git a/llvm/test/CodeGen/SystemZ/swift-return.ll b/llvm/test/CodeGen/SystemZ/swift-return.ll index 7cc432b322f0..af80a508ae84 100644 --- a/llvm/test/CodeGen/SystemZ/swift-return.ll +++ b/llvm/test/CodeGen/SystemZ/swift-return.ll @@ -39,9 +39,8 @@ declare swiftcc { i16, i8 } @gen(i32) ; in memroy. The caller provides space for the return value and passes ; the address in %r2. The first input argument will be in %r3. ; CHECK-LABEL: test2: -; CHECK: lr %[[REG1:r[0-9]+]], %r2 +; CHECK: lr %r3, %r2 ; CHECK-DAG: la %r2, 160(%r15) -; CHECK-DAG: lr %r3, %[[REG1]] ; CHECK: brasl %r14, gen2 ; CHECK: l %r2, 160(%r15) ; CHECK: a %r2, 164(%r15) diff --git a/llvm/test/CodeGen/SystemZ/swifterror.ll b/llvm/test/CodeGen/SystemZ/swifterror.ll index 092a7b133f13..eac84ef3ef7c 100644 --- a/llvm/test/CodeGen/SystemZ/swifterror.ll +++ b/llvm/test/CodeGen/SystemZ/swifterror.ll @@ -34,11 +34,11 @@ define float @caller(i8* %error_ref) { ; CHECK: lgr %r[[REG1:[0-9]+]], %r2 ; CHECK: lghi %r9, 0 ; CHECK: brasl %r14, foo -; CHECK: cgijlh %r9, 0, +; CHECK: %r2, %r9 +; CHECK: jlh ; Access part of the error object and save it to error_ref -; CHECK: lb %r[[REG2:[0-9]+]], 8(%r9) +; CHECK: lb %r[[REG2:[0-9]+]], 8(%r2) ; CHECK: stc %r[[REG2]], 0(%r[[REG1]]) -; CHECK: lgr %r2, %r9 ; CHECK: brasl %r14, free ; CHECK-O0-LABEL: caller: ; CHECK-O0: lghi %r9, 0 @@ -246,11 +246,10 @@ define float @caller3(i8* %error_ref) { ; CHECK: lhi %r3, 1 ; CHECK: lghi %r9, 0 ; CHECK: brasl %r14, foo_sret -; CHECK: cgijlh %r9, 0, +; CHECK: jlh ; Access part of the error object and save it to error_ref -; CHECK: lb %r0, 8(%r9) +; CHECK: lb %r0, 8(%r2) ; CHECK: stc %r0, 0(%r[[REG1]]) -; CHECK: lgr %r2, %r9 ; CHECK: brasl %r14, free ; CHECK-O0-LABEL: caller3: @@ -296,21 +295,21 @@ define float @caller_with_multiple_swifterror_values(i8* %error_ref, i8* %error_ ; The first swifterror value: ; CHECK: lghi %r9, 0 ; CHECK: brasl %r14, foo -; CHECK: cgijlh %r9, 0, +; CHECK: ltgr %r2, %r9 +; CHECK: jlh ; Access part of the error object and save it to error_ref -; CHECK: lb %r0, 8(%r9) +; CHECK: lb %r0, 8(%r2) ; CHECK: stc %r0, 0(%r[[REG1]]) -; CHECK: lgr %r2, %r9 ; CHECK: brasl %r14, free ; The second swifterror value: ; CHECK: lghi %r9, 0 ; CHECK: brasl %r14, foo -; CHECK: cgijlh %r9, 0, +; CHECK: ltgr %r2, %r9 +; CHECK: jlh ; Access part of the error object and save it to error_ref -; CHECK: lb %r0, 8(%r9) +; CHECK: lb %r0, 8(%r2) ; CHECK: stc %r0, 0(%r[[REG2]]) -; CHECK: lgr %r2, %r9 ; CHECK: brasl %r14, free ; CHECK-O0-LABEL: caller_with_multiple_swifterror_values: