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

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 10 08:17:46 PDT 2017


qcolombet 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 {
----------------
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 :)


https://reviews.llvm.org/D38128





More information about the llvm-commits mailing list