[PATCH] D31123: RegisterPressure: Add operators to RegisterMaskPair

Axel Davy via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 19 15:28:05 PDT 2017


I'm not familiar with std::transform, but it looks like one element in 
the source
can only give one element in the destination, whereas here you can have 
one element
decomposing into several ones.

On 19/03/2017 23:22, Daniel Berlin 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/a1cde2fc/attachment.html>


More information about the llvm-commits mailing list