diff mbox series

[04/13] arm: Add GENERAL_AND_VPR_REGS regclass

Message ID 20210907091531.1034282-5-christophe.lyon@foss.st.com
State New
Headers show
Series ARM/MVE use vectors of boolean for predicates | expand

Commit Message

Christophe Lyon Sept. 7, 2021, 9:15 a.m. UTC 
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.

Comments

Richard Earnshaw Sept. 7, 2021, 9:42 a.m. UTC | #1 
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?

R.


>   
>   #define FP_SYSREGS \
>
Christophe Lyon Sept. 7, 2021, 12:05 p.m. UTC | #2 
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?


>
> R.
>
>
>>     #define FP_SYSREGS \
>>
Richard Earnshaw Sept. 7, 2021, 1:35 p.m. UTC | #3 
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.

R.

> 
> 
>>
>> R.
>>
>>
>>>     #define FP_SYSREGS \
>>>
Christophe Lyon Sept. 8, 2021, 7:48 a.m. UTC | #4 
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.

Thanks,

Christophe


>
> R.
>
>>
>>
>>>
>>> R.
>>>
>>>
>>>>     #define FP_SYSREGS \
>>>>
Kyrylo Tkachov Sept. 28, 2021, 11:18 a.m. UTC | #5 
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.

Thanks,
Kyrill

> 
> Thanks,
> 
> Christophe
> 
> 
> >
> > R.
> >
> >>
> >>
> >>>
> >>> R.
> >>>
> >>>
> >>>>     #define FP_SYSREGS \
> >>>>
Christophe Lyon Sept. 28, 2021, 1:32 p.m. UTC | #6 
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 :-)

Thanks,

Christophe


>
> Thanks,
> Kyrill
>
>> Thanks,
>>
>> Christophe
>>
>>
>>> R.
>>>
>>>>
>>>>> R.
>>>>>
>>>>>
>>>>>>      #define FP_SYSREGS \
>>>>>>
Christophe Lyon Oct. 11, 2021, 12:44 p.m. UTC | #7 
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 \
>>>>>>>
From c57fb3fc853d8bf04f589682f03e9d3baac2dbd5 Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@foss.st.com>
Date: Thu, 26 Aug 2021 16:01:58 +0000
Subject: [PATCH v2 04/14] arm: Add GENERAL_AND_VPR_REGS regclass
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 015299c1534..eae1b1cd0fb 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,6 +1345,7 @@ enum reg_class
   { 0x00000000, 0x00000000, 0x00000000, 0x00000040 }, /* SFP_REG */	\
   { 0x00000000, 0x00000000, 0x00000000, 0x00000080 }, /* AFP_REG */	\
   { 0x00000000, 0x00000000, 0x00000000, 0x00000400 }, /* VPR_REG.  */	\
+  { 0x00005FFF, 0x00000000, 0x00000000, 0x00000400 }, /* GENERAL_AND_VPR_REGS.  */ \
   { 0xFFFF7FFF, 0xFFFFFFFF, 0xFFFFFFFF, 0x0000000F }  /* ALL_REGS.  */	\
 }
diff mbox series

Patch

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.  */	\
 }
 
 #define FP_SYSREGS \