[PATCH] D13185: Scheduler / Regalloc: use unique_ptr[] instead of std::vector

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 25 17:32:31 PDT 2015


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?)


> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150925/03652582/attachment-0001.html>


More information about the llvm-commits mailing list