[PATCH] D31123: RegisterPressure: Add operators to RegisterMaskPair

[Daniel Berlin via llvm-commits]

On Sun, Mar 19, 2017 at 3:13 PM, Axel Davy <axel.davy at ens.fr> wrote:

> The patch is on top of other patches not yet merged.
>
> You can find a branch with all the patches here:
> https://github.com/axeldavy/llvm/commits/llvm_upstream
>
> getPairsForRegs is not equivalent to std::copy.
>

Okay, then it's definitely equivalent to std::transform  ;)
(though note: It looks nearly trivial to define a subclass of the register
mask pair that makes *all* those calls std::copy)


> The name is probably poorly choosen,
> I'm open to suggestions.
>
> Yours,
>
> Axel
>
>
> On 19/03/2017 23:10, Daniel Berlin wrote:
>
> This code has a number of issues (using copying all over the place) that
> is likely causing your issue.
> For example:
> // Idem for a list of Reg/Mask
>   SmallVector<RegisterMaskPair, 8> getPairsForRegs(
>     const SmallVector<RegisterMaskPair, 8> Regs);
>
>
> You almost certainly meant this to be a const reference to Regs.
> etc
>
> Actually, staring at/ this harder, that function is just equivalent to
> std::copy, no?
>
> I tried to patch it into a client, but it does not patch cleanly.
> If you rebase it, i'll take a closer look.
>
>
> On Sun, Mar 19, 2017 at 2:22 PM, Axel Davy <axel.davy at ens.fr> wrote:
>
>> Here is the code that makes use of this:
>>
>> https://reviews.llvm.org/D31124
>>
>> Axel
>>
>>
>> On 19/03/2017 22:16, Daniel Berlin wrote:
>>
>> Can you give the actual code you are trying to use?
>> It should work fine in, say, densemap, if you define the densemapinfo for
>> it.
>>
>> Past that, without seeing exactly how you are trying to use it, it is
>> hard to say what the problem is.
>>
>> I would venture a guess if you are doing it in a way that it is trying to
>> default construct these, you are probably not doing it right :)
>>
>>
>> On Sun, Mar 19, 2017 at 12:44 PM, Axel Davy via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> This is similar to just allocating the structure.
>>>
>>> Somehow to use maps, it complained without this constructor.
>>> I don't think however any possible initialization value would make sense.
>>>
>>> Perhaps UINT_MAX for Reg and 0 for LaneMask ? To clearly indicate there
>>> is error
>>> if you see these in logs ?
>>>
>>> Axel
>>>
>>>
>>> On 19/03/2017 20:23, Stanislav Mekhanoshin via Phabricator wrote:
>>>
>>>> rampitec added inline comments.
>>>>
>>>>
>>>> ================
>>>> Comment at: include/llvm/CodeGen/RegisterPressure.h:45
>>>> +
>>>> +  RegisterMaskPair() {}
>>>> +
>>>> ----------------
>>>> That is undesirable to have a constructor which does not initialize
>>>> fields.
>>>>
>>>>
>>>> Repository:
>>>>    rL LLVM
>>>>
>>>> https://reviews.llvm.org/D31123
>>>>
>>>>
>>>>
>>>>
>>> _______________________________________________
>>> 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/20170319/c458426c/attachment.html>