[PATCH] rs6000: Fix constraint v with rs6000_constraints[RS6000_CONSTRAINT_v]

Kewen.Lin linkw@linux.ibm.com
Thu Jan 13 03:56:00 GMT 2022


on 2022/1/13 上午11:44, David Edelsohn wrote:
> On Wed, Jan 12, 2022 at 10:38 PM Kewen.Lin <linkw@linux.ibm.com> wrote:
>>
>> Hi David,
>>
>> on 2022/1/13 上午11:07, David Edelsohn wrote:
>>> On Wed, Jan 12, 2022 at 8:56 PM Kewen.Lin <linkw@linux.ibm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> This patch is to fix register constraint v with
>>>> rs6000_constraints[RS6000_CONSTRAINT_v] instead of ALTIVEC_REGS,
>>>> just like some other existing register constraints with
>>>> RS6000_CONSTRAINT_*.
>>>>
>>>> I happened to see this and hope it's not intentional and just
>>>> got neglected.
>>>>
>>>> Bootstrapped and regtested on powerpc64le-linux-gnu P9 and
>>>> powerpc64-linux-gnu P8.
>>>>
>>>> Is it ok for trunk?
>>>
>>> Why do you want to make this change?
>>>
>>> rs6000_constraints[RS6000_CONSTRAINT_v] = ALTIVEC_REGS;
>>>
>>> but all of the patterns that use a "v" constraint are (or should be)
>>> protected by TARGET_ALTIVEC, or some final condition that only is
>>> active for TARGET_ALTIVEC.  The other constraints are conditionally
>>> set because they can be used in a pattern with multiple alternatives
>>> where the pattern itself is active but some of the constraints
>>> correspond to NO_REGS when some instruction variants for VSX is not
>>> enabled.
>>>
>>
>> Good point!  Thanks for the explanation.
>>
>>> The change isn't wrong, but it doesn't correct a bug and provides no
>>> additional benefit nor clarty that I can see.
>>>
>>
>> The original intention is to make it consistent with the other existing
>> register constraints with RS6000_CONSTRAINT_*, otherwise it looks a bit
>> weird (like was neglected).  After you clarified above, RS6000_CONSTRAINT_v
>> seems useless at all in the current framework.  Do you prefer to remove
>> it to avoid any confusions instead?
> 
> It's used in the reg_class, so there may be some heuristic in the GCC
> register allocator that cares about the number of registers available
> for the target.  rs6000_constraints[RS6000_CONSTRAINT_v] is defined
> conditionally, so it seems best to leave it as is.
> 

I may miss something, but I didn't find it's used for the above purposes.
If it's best to leave it as is, the proposed patch seems to offer better
readability.

BR,
Kewen
> Thanks, David
> 



More information about the Gcc-patches mailing list