[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 18:03:50 PDT 2015
Attached is the resulting -view-cfg of the -O2 IR.
On Fri, Sep 25, 2015 at 5:58 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>
> 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/36c02467/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cfg_ZNSt3__16vectorIPcNS_9allocatorIS1_EEE8__appendEmRKS1_-13aab5.pdf
Type: application/pdf
Size: 30562 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/36c02467/attachment.pdf>
More information about the llvm-commits
mailing list