[PATCH] D31123: RegisterPressure: Add operators to RegisterMaskPair

[Axel Davy via llvm-commits]

So that's how to do it :-)

I guess it's too late to add it, but I'll think of it next time, thanks !

On 19/03/2017 23:26, Daniel Berlin wrote:
> Also, if you add the text "depends on <reviewnumber>" to the summary 
> of the patch, and phab will link them.
>
>
>
> On Sun, Mar 19, 2017 at 3:22 PM, Daniel Berlin <dberlin at dberlin.org 
> <mailto:dberlin at dberlin.org>> wrote:
>
>
>
>     On Sun, Mar 19, 2017 at 3:13 PM, Axel Davy <axel.davy at ens.fr
>     <mailto: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
>         <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
>>         <mailto:axel.davy at ens.fr>> wrote:
>>
>>             Here is the code that makes use of this:
>>
>>             https://reviews.llvm.org/D31124
>>             <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
>>>             <mailto: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
>>>                     <https://reviews.llvm.org/D31123>
>>>
>>>
>>>
>>>
>>>                 _______________________________________________
>>>                 llvm-commits mailing list
>>>                 llvm-commits at lists.llvm.org
>>>                 <mailto:llvm-commits at lists.llvm.org>
>>>                 http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>                 <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/ac8228f6/attachment.html>