[PATCH] D38128: Handle COPYs of physregs better (regalloc hints)

Jonas Paulsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 11 04:02:26 PDT 2017


jonpa requested review of this revision.
jonpa added inline comments.


================
Comment at: include/llvm/CodeGen/MachineRegisterInfo.h:755
+  /// register allocation hints for VReg.
+  const std::pair<unsigned, SmallVector<unsigned, 4>>
+  &getRegAllocationHints(unsigned VReg) const {
----------------
qcolombet wrote:
> jonpa wrote:
> > qcolombet wrote:
> > > Could we use SmallVectorImpl to not leak the size of the vector?
> > I'm not sure I tried all atlernatives, but there seems to be some constructor missing or something, because I get:
> > 
> > 
> > ```
> > include/llvm/ADT/IndexedMap.h:42:29: error: no matching function for call to ‘std::pair<unsigned int, llvm::SmallVectorImpl<unsigned int> >::pair()’
> >      IndexedMap() : nullVal_(T()) {}
> >                              ^~~
> > ```
> > I simply did
> > 
> > 
> > ```
> > @ -89,11 +89,11 @@ private:
> >    /// type and hints vector making up the allocation hints. Only the first
> >    /// hint may be target specific, and in that case this is reflected by the
> >    /// first member of the pair being non-zero. If the hinted register is
> >    /// virtual, it means the allocator should prefer the physical register
> >    /// allocated to it if any.
> > -  IndexedMap<std::pair<unsigned, SmallVector<unsigned, 4>>,
> > +  IndexedMap<std::pair<unsigned, SmallVectorImpl<unsigned>>,
> >               VirtReg2IndexFunctor> RegAllocHints;
> >  
> >    /// PhysRegUseDefLists - This is an array of the head of the use/def list for
> >    /// physical registers.
> >    std::unique_ptr<MachineOperand *[]> PhysRegUseDefLists;
> > @@ -750,11 +750,11 @@ public:
> >      return Hint.first ? 0 : Hint.second;
> >    }
> >  
> >    /// getRegAllocationHints - Return a reference to the vector of all
> >    /// register allocation hints for VReg.
> > -  const std::pair<unsigned, SmallVector<unsigned, 4>>
> > +  const std::pair<unsigned, SmallVectorImpl<unsigned>>
> >    &getRegAllocationHints(unsigned VReg) const {
> >      assert(TargetRegisterInfo::isVirtualRegister(VReg));
> >      return RegAllocHints[VReg];
> >    }
> > ```
> > 
> > BTW, it would be nice to know how SmallVector would leak if using it here...
> By leak I mean exposing implementation details to users :)
Aah, I see :-)


https://reviews.llvm.org/D38128





More information about the llvm-commits mailing list