[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 19:23:46 PDT 2015
Yeah, the optimizer seems to be failing somewhere. One of the first things
is that we don't actually fully inline the previous example (so we can't
constant fold the store of 0), also we have code for the "shrinking" cases.
So to simplify, this code (using libc++ <vector>)
```
#include <vector>
void blarg(std::vector<char *> &V) {
std::vector<char*> VNew(1000, nullptr);
V.swap(VNew);
}
```
We get the attached code from:
~/pg/release/bin/clang++ -I ~/pg/libcxx/include -fno-exceptions
-fno-unroll-loops -c -O2 testvectorresize.cpp -mllvm
-inline-threshold=10000 -emit-llvm -o - | ~/pg/release/bin/opt -S -view-cfg
I've also attached O0 IR in case somebody wants to take a look at it.
On Fri, Sep 25, 2015 at 6:03 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 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/d49a6859/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cfg_Z5blargRNSt3__16vectorIPcNS_9allocatorIS1_EEEE-7c149e.pdf
Type: application/pdf
Size: 25046 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/d49a6859/attachment.pdf>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ir.ll
Type: application/octet-stream
Size: 4275 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/d49a6859/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: O0ir.ll
Type: application/octet-stream
Size: 60901 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/d49a6859/attachment-0001.obj>
More information about the llvm-commits
mailing list