Re: [PATCH] rtl: Forward declare rtx_code

[Richard Sandiford <richard.sandiford@arm.com>]

"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com> writes:
> On 23/08/2023 16:49, Richard Sandiford via Gcc-patches wrote:
>> Richard Earnshaw via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>>> Now that we require C++ 11, we can safely forward declare rtx_code
>>> so that we can use it in target hooks.
>>>
>>> gcc/ChangeLog
>>> 	* coretypes.h (rtx_code): Add forward declaration.
>>> 	* rtl.h (rtx_code): Make compatible with forward declaration.
>>> ---
>>>  gcc/coretypes.h | 4 ++++
>>>  gcc/rtl.h       | 2 +-
>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gcc/coretypes.h b/gcc/coretypes.h
>>> index ca8837cef67..51e55559ce0 100644
>>> --- a/gcc/coretypes.h
>>> +++ b/gcc/coretypes.h
>>> @@ -100,6 +100,10 @@ struct gimple;
>>>  typedef gimple *gimple_seq;
>>>  struct gimple_stmt_iterator;
>>>  
>>> +/* Forward declare rtx_code, so that we can use it in target hooks without
>>> +   needing to pull in rtl.h.  */
>>> +enum rtx_code : unsigned;
>>> +
>>>  /* Forward decls for leaf gimple subclasses (for individual gimple codes).
>>>     Keep this in the same order as the corresponding codes in gimple.def.  */
>>>  
>>> diff --git a/gcc/rtl.h b/gcc/rtl.h
>>> index e1c51156f90..0e9491b89b4 100644
>>> --- a/gcc/rtl.h
>>> +++ b/gcc/rtl.h
>>> @@ -45,7 +45,7 @@ class predefined_function_abi;
>>>  /* Register Transfer Language EXPRESSIONS CODES */
>>>  
>>>  #define RTX_CODE	enum rtx_code
>>> -enum rtx_code  {
>>> +enum rtx_code : unsigned {
>>>  
>>>  #define DEF_RTL_EXPR(ENUM, NAME, FORMAT, CLASS)   ENUM ,
>>>  #include "rtl.def"		/* rtl expressions are documented here */
>> 
>> Given:
>> 
>>   #define RTX_CODE_BITSIZE 8
>> 
>> there might be some value in making it uint8_t rather than unsigned.
>> Preapproved if you agree.
>> 
>> But the patch as posted is a strict improvement over the status quo,
>> so it's also OK as-is.
>> 
>> Thanks,
>> Richard
>
> I did think about that, but there were two reasons for not doing so:
> - it presumes we would never want more than 8 bits for rtx_code (well, not quite, 
> but it would make it more work to change this).

The rtx_def structure itself provides a significant barrier to that though.

If we ever think that we need to represent more than 256 separate
operations, I think the natural way would be to treat the less well-used
ones in a similar way to unspecs.

> - it would probably lead to more zero-extension operations happening in the compiler

Yeah, that's true.  The upside though is that we could then declare
arrays of codes directly, without having to resort to "unsigned char"
tricks.  That's unlikely to help codes much, but the same principle
would apply to modes, which are more frequently put into arrays.

E.g. one of the issues with bumping the machine_mode bitfield from 8 to
16 bits was finding all the places where "unsigned char" was used to
hold modes, and changing them to "unsigned short".  If machine_mode was
instead the "right" size, we could just call a spade a spade.

But like I say, that's mostly reasoning by analogy rather than because
the size of rtx_code itself is important.

Richard