[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