On 28/09/2021 15:32, Christophe LYON via Gcc-patches wrote:
>
> On 28/09/2021 13:18, Kyrylo Tkachov wrote:
>> Hi Christophe,
>>
>>> -----Original Message-----
>>> From: Gcc-patches <gcc-patches-
>>> bounces+kyrylo.tkachov=arm.com@gcc.gnu.org> On Behalf Of Christophe
>>> LYON via Gcc-patches
>>> Sent: 08 September 2021 08:49
>>> To: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>; gcc-
>>> patches@gcc.gnu.org
>>> Subject: Re: [PATCH 04/13] arm: Add GENERAL_AND_VPR_REGS regclass
>>>
>>>
>>> On 07/09/2021 15:35, Richard Earnshaw wrote:
>>>>
>>>> On 07/09/2021 13:05, Christophe LYON wrote:
>>>>> On 07/09/2021 11:42, Richard Earnshaw wrote:
>>>>>>
>>>>>> On 07/09/2021 10:15, Christophe Lyon via Gcc-patches wrote:
>>>>>>> At some point during the development of this patch series, it 
>>>>>>> appeared
>>>>>>> that in some cases the register allocator wants “VPR or general”
>>>>>>> rather than “VPR or general or FP” (which is the same thing as
>>>>>>> ALL_REGS).  The series does not seem to require this anymore, 
>>>>>>> but it
>>>>>>> seems to be a good thing to do anyway, to give the register 
>>>>>>> allocator
>>>>>>> more freedom.
>>>>>>>
>>>>>>> 2021-09-01  Christophe Lyon <christophe.lyon@foss.st.com>
>>>>>>>
>>>>>>>      gcc/
>>>>>>>      * config/arm/arm.h (reg_class): Add GENERAL_AND_VPR_REGS.
>>>>>>>      (REG_CLASS_NAMES): Likewise.
>>>>>>>      (REG_CLASS_CONTENTS): Likewise. Add VPR_REG to ALL_REGS.
>>>>>>>
>>>>>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>>>>>>> index 015299c1534..fab39d05916 100644
>>>>>>> --- a/gcc/config/arm/arm.h
>>>>>>> +++ b/gcc/config/arm/arm.h
>>>>>>> @@ -1286,6 +1286,7 @@ enum reg_class
>>>>>>>      SFP_REG,
>>>>>>>      AFP_REG,
>>>>>>>      VPR_REG,
>>>>>>> +  GENERAL_AND_VPR_REGS,
>>>>>>>      ALL_REGS,
>>>>>>>      LIM_REG_CLASSES
>>>>>>>    };
>>>>>>> @@ -1315,6 +1316,7 @@ enum reg_class
>>>>>>>      "SFP_REG",        \
>>>>>>>      "AFP_REG",        \
>>>>>>>      "VPR_REG",        \
>>>>>>> +  "GENERAL_AND_VPR_REGS", \
>>>>>>>      "ALL_REGS"        \
>>>>>>>    }
>>>>>>>    @@ -1343,7 +1345,8 @@ enum reg_class
>>>>>>>      { 0x00000000, 0x00000000, 0x00000000, 0x00000040 }, /* SFP_REG
>>>>>>> */    \
>>>>>>>      { 0x00000000, 0x00000000, 0x00000000, 0x00000080 }, /* AFP_REG
>>>>>>> */    \
>>>>>>>      { 0x00000000, 0x00000000, 0x00000000, 0x00000400 }, /* 
>>>>>>> VPR_REG.
>>>>>>> */    \
>>>>>>> -  { 0xFFFF7FFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x0000000F } /* ALL_REGS.
>>>>>>> */    \
>>>>>>> +  { 0x00005FFF, 0x00000000, 0x00000000, 0x00000400 }, /*
>>>>>>> GENERAL_AND_VPR_REGS.  */ \
>>>>>>> +  { 0xFFFF7FFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x0000040F } /* ALL_REGS.
>>>>>>> */    \
>>>>>>>    }
>>>>>> You've changed the definition of ALL_REGS here (to include VPR_REG),
>>>>>> but not really explained why.  Is that the source of the underlying
>>>>>> issue with the 'appeared' you mention?
>>>>>
>>>>> I first added VPR_REG to ALL_REGS, but Richard Sandiford suggested I
>>>>> create a new GENERAL_AND_VPR_REGS that would be more restrictive. I
>>>>> did not remove VPR_REG from ALL_REGS because I thought it was an
>>>>> omission: shouldn't ALL_REGS contain all registers?
>>>> Surely that should be a separate patch then.
>>> OK, I can remove that line from this patch and make a separate 
>>> one-liner
>>> for ALL_REGS.
>> Did you end up sending that patch out? (Sorry, I may have missed it 
>> in my archive).
>> This patch to add GENERAL_AND_VPR_REGS is okay with the ALL_REGS 
>> change separated out.
>
> No I didn't send it yet: I suspect there will be iterations on the 
> next patches in the series, this small change alone wasn't worth 
> sending a v2 :-)
>
Here is the patch now split into two parts.

Christophe


> Thanks,
>
> Christophe
>
>
>>
>> Thanks,
>> Kyrill
>>
>>> Thanks,
>>>
>>> Christophe
>>>
>>>
>>>> R.
>>>>
>>>>>
>>>>>> R.
>>>>>>
>>>>>>
>>>>>>>      #define FP_SYSREGS \
>>>>>>>