[PATCH] D13185: Scheduler / Regalloc: use unique_ptr[] instead of std::vector
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 25 17:58:18 PDT 2015
On Fri, Sep 25, 2015 at 5:32 PM, David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
>
>
> On Fri, Sep 25, 2015 at 5:28 PM, escha via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> escha created this revision.
>> escha added reviewers: MatzeB, arsenm, resistor.
>> escha added a subscriber: llvm-commits.
>> escha set the repository for this revision to rL LLVM.
>>
>> std::vector.resize() is actually about 2-3x slower than using unique_ptr,
>> at least for me; it doesn't compile to bzero because the resize() in std::
>> consists of repeated appending (agh).
>
>
> Wait, what? Which standard library has that implementation? (is it
> necessary/implied by the standard, or just a poorly performing
> implementation that can/should be fixed?)
>
For libcxx at least it boils down to:
http://reviews.llvm.org/P126
For __construct_at_end:
`// Precondition: size() + __n <= capacity()`
So it isn't repeated push_back's. We are just failing to optimize it (or
maybe something is weird inside vector?). Marshall, any ideas?
We do end up with some really funky looking code though:
http://reviews.llvm.org/P127
```
#include <vector>
void blarg(std::vector<char *> &V) {
V.resize(1000, nullptr);
}
```
(one thing to note is that the "shrinking" case also has to be handled
(i.e. if V.size() was >1000 on entry to the function))
The O3 IR is scary looking: http://reviews.llvm.org/P128
-- Sean Silva
>
>
>> On our out of tree target with a huge number of registers, this patch
>> actually saves 1% of compile time. It should help significantly on AMDGPU
>> as well, though likely a factor less due to them not having quite the same
>> level of ridiculous register counts.
>>
>> In general, we probably shouldn't use the heavyweight-ness of std::vector
>> for constant-size arrays. There's really no reason to.
>>
>> Repository:
>> rL LLVM
>>
>> http://reviews.llvm.org/D13185
>>
>> Files:
>> include/llvm/CodeGen/MachineRegisterInfo.h
>> lib/CodeGen/MachineRegisterInfo.cpp
>> lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
>>
>> Index: lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
>> ===================================================================
>> --- lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
>> +++ lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
>> @@ -141,8 +141,8 @@
>> /// that are "live". These nodes must be scheduled before any other
>> nodes that
>> /// modifies the registers can be scheduled.
>> unsigned NumLiveRegs;
>> - std::vector<SUnit*> LiveRegDefs;
>> - std::vector<SUnit*> LiveRegGens;
>> + std::unique_ptr<SUnit*[]> LiveRegDefs;
>> + std::unique_ptr<SUnit*[]> LiveRegGens;
>>
>> // Collect interferences between physical register use/defs.
>> // Each interference is an SUnit and set of physical registers.
>> @@ -328,8 +328,8 @@
>> NumLiveRegs = 0;
>> // Allocate slots for each physical register, plus one for a special
>> register
>> // to track the virtual resource of a calling sequence.
>> - LiveRegDefs.resize(TRI->getNumRegs() + 1, nullptr);
>> - LiveRegGens.resize(TRI->getNumRegs() + 1, nullptr);
>> + LiveRegDefs.reset(new SUnit*[TRI->getNumRegs() + 1]());
>> + LiveRegGens.reset(new SUnit*[TRI->getNumRegs() + 1]());
>> CallSeqEndForStart.clear();
>> assert(Interferences.empty() && LRegsMap.empty() && "stale
>> Interferences");
>>
>> @@ -1218,7 +1218,7 @@
>> /// CheckForLiveRegDef - Return true and update live register vector if
>> the
>> /// specified register def of the specified SUnit clobbers any "live"
>> registers.
>> static void CheckForLiveRegDef(SUnit *SU, unsigned Reg,
>> - std::vector<SUnit*> &LiveRegDefs,
>> + std::unique_ptr<SUnit*[]> &LiveRegDefs,
>> SmallSet<unsigned, 4> &RegAdded,
>> SmallVectorImpl<unsigned> &LRegs,
>> const TargetRegisterInfo *TRI) {
>> @@ -1240,11 +1240,12 @@
>> /// CheckForLiveRegDefMasked - Check for any live physregs that are
>> clobbered
>> /// by RegMask, and add them to LRegs.
>> static void CheckForLiveRegDefMasked(SUnit *SU, const uint32_t *RegMask,
>> - std::vector<SUnit*> &LiveRegDefs,
>> + std::unique_ptr<SUnit*[]>
>> &LiveRegDefs,
>> SmallSet<unsigned, 4> &RegAdded,
>> - SmallVectorImpl<unsigned> &LRegs) {
>> + SmallVectorImpl<unsigned> &LRegs,
>> + unsigned NumRegs) {
>> // Look at all live registers. Skip Reg0 and the special CallResource.
>> - for (unsigned i = 1, e = LiveRegDefs.size()-1; i != e; ++i) {
>> + for (unsigned i = 1, e = NumRegs; i != e; ++i) {
>> if (!LiveRegDefs[i]) continue;
>> if (LiveRegDefs[i] == SU) continue;
>> if (!MachineOperand::clobbersPhysReg(RegMask, i)) continue;
>> @@ -1328,7 +1329,8 @@
>> }
>> }
>> if (const uint32_t *RegMask = getNodeRegMask(Node))
>> - CheckForLiveRegDefMasked(SU, RegMask, LiveRegDefs, RegAdded,
>> LRegs);
>> + CheckForLiveRegDefMasked(SU, RegMask, LiveRegDefs, RegAdded, LRegs,
>> + TRI->getNumRegs());
>>
>> const MCInstrDesc &MCID = TII->get(Node->getMachineOpcode());
>> if (!MCID.ImplicitDefs)
>> Index: lib/CodeGen/MachineRegisterInfo.cpp
>> ===================================================================
>> --- lib/CodeGen/MachineRegisterInfo.cpp
>> +++ lib/CodeGen/MachineRegisterInfo.cpp
>> @@ -27,12 +27,11 @@
>> MachineRegisterInfo::MachineRegisterInfo(const MachineFunction *MF)
>> : MF(MF), TheDelegate(nullptr), IsSSA(true), TracksLiveness(true),
>> TracksSubRegLiveness(false) {
>> + unsigned NumRegs = getTargetRegisterInfo()->getNumRegs();
>> VRegInfo.reserve(256);
>> RegAllocHints.reserve(256);
>> - UsedPhysRegMask.resize(getTargetRegisterInfo()->getNumRegs());
>> -
>> - // Create the physreg use/def lists.
>> - PhysRegUseDefLists.resize(getTargetRegisterInfo()->getNumRegs(),
>> nullptr);
>> + UsedPhysRegMask.resize(NumRegs);
>> + PhysRegUseDefLists.reset(new MachineOperand*[NumRegs]());
>> }
>>
>> /// setRegClass - Set the register class of the specified virtual
>> register.
>> Index: include/llvm/CodeGen/MachineRegisterInfo.h
>> ===================================================================
>> --- include/llvm/CodeGen/MachineRegisterInfo.h
>> +++ include/llvm/CodeGen/MachineRegisterInfo.h
>> @@ -73,7 +73,7 @@
>>
>> /// PhysRegUseDefLists - This is an array of the head of the use/def
>> list for
>> /// physical registers.
>> - std::vector<MachineOperand *> PhysRegUseDefLists;
>> + std::unique_ptr<MachineOperand *[]> PhysRegUseDefLists;
>>
>> /// getRegUseDefListHead - Return the head pointer for the register
>> use/def
>> /// list for the specified virtual or physical register.
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/b2d4eb21/attachment.html>
More information about the llvm-commits
mailing list