mbox series

[0/5] aarch64: Remove aarch64_field_kind indirection

Message ID b07af6ec-a74a-8c90-a2c7-38c03664b882@e124511.cambridge.arm.com
Headers
Series aarch64: Remove aarch64_field_kind indirection |

Message

Alice Carlotti May 20, 2026, 12:08 p.m. UTC 
  This patch series remove the aarch64_field_kind enum, and replaces uses of the
enum with direct use of aarch64_field structs instead.  I think this improves
readability overall, and might also be a marginal performance improvement.

Passing aarch64_field as function arguments causes GCC to emit this note:

  note: parameter passing for argument of type ‘aarch64_field’
{aka ‘struct aarch64_field’} changed in GCC 9.1

Is this a problem?  Should I find some way to avoid this note being emitted?


As an interim measure, I've retained some of the aarch64_field_kind enum names
as macros that expand to corresponding the aarch64_field value.  I've
deliberately chosen not to inline all of these remaining uses immediately, as
in many cases I intend to either add comments or rework the surrounding code
first.  After this patch series, there would be 76 macros remaining (not
counting the FLD_CONST_* macros), out of 216 original enum values.


Does this approach look ok?

Thanks,
Alice

Comments

Richard Earnshaw May 29, 2026, 3 p.m. UTC | #1 
On 20/05/2026 13:08, Alice Carlotti wrote:
> This patch series remove the aarch64_field_kind enum, and replaces uses of the
> enum with direct use of aarch64_field structs instead.  I think this improves
> readability overall, and might also be a marginal performance improvement.
> 
> Passing aarch64_field as function arguments causes GCC to emit this note:
> 
>   note: parameter passing for argument of type ‘aarch64_field’
> {aka ‘struct aarch64_field’} changed in GCC 9.1

That depends on which files generate this warning.  Does this function form part of an API that would be exported if/when libopcodes is made into an installed library?

If not, then we probably don't care.  We also don't care if it only occurs when this is used as part of the code used to build the auto-generated code.

I think there's an option to gcc to suppress the warning (-Wabi?).  Perhaps one way to suppress the warning would be with a pragma; that would be preferable to disabling it on the command line, since that might lead to other cases that do matter not being reported.


> 
> Is this a problem?  Should I find some way to avoid this note being emitted?
> 
> 
> As an interim measure, I've retained some of the aarch64_field_kind enum names
> as macros that expand to corresponding the aarch64_field value.  I've
> deliberately chosen not to inline all of these remaining uses immediately, as
> in many cases I intend to either add comments or rework the surrounding code
> first.  After this patch series, there would be 76 macros remaining (not
> counting the FLD_CONST_* macros), out of 216 original enum values.
> 
> 
> Does this approach look ok?
> 
> Thanks,
> Alice

R.
Alice Carlotti June 10, 2026, 2:17 p.m. UTC | #2 
On Fri, May 29, 2026 at 04:00:14PM +0100, Richard Earnshaw (foss) wrote:
> On 20/05/2026 13:08, Alice Carlotti wrote:
> > This patch series remove the aarch64_field_kind enum, and replaces uses of the
> > enum with direct use of aarch64_field structs instead.  I think this improves
> > readability overall, and might also be a marginal performance improvement.
> > 
> > Passing aarch64_field as function arguments causes GCC to emit this note:
> > 
> >   note: parameter passing for argument of type ‘aarch64_field’
> > {aka ‘struct aarch64_field’} changed in GCC 9.1
> 
> That depends on which files generate this warning.  Does this function form part of an API that would be exported if/when libopcodes is made into an installed library?

The affected functions are gen_sub_field, insert_field, insert_fields and
extract_field.  They are static inline functions that are only used internally.
The warning is actually inaccurate, and is only emitted in GCC 9 and GCC 10.
It was subsequently tightened in r11-8226-g49813aad3292f7.

> 
> If not, then we probably don't care.  We also don't care if it only occurs when this is used as part of the code used to build the auto-generated code.
> 
> I think there's an option to gcc to suppress the warning (-Wabi?).  Perhaps one way to suppress the warning would be with a pragma; that would be preferable to disabling it on the command line, since that might lead to other cases that do matter not being reported.

I'd be inclined to just leave this as is, given that it only affects old GCC
versions, but if we wanted to mask the warning we could add something like:

+ #if GCC_VERSION >= 9000
+ #pragma GCC diagnostic push
+ #pragma GCC diagnostic ignored "-Wpsabi"
+ #endif
...
+ #if GCC_VERSION >= 9000
+ #pragma GCC diagnostic pop
+ #endif


Alice

> 
> 
> > 
> > Is this a problem?  Should I find some way to avoid this note being emitted?
> > 
> > 
> > As an interim measure, I've retained some of the aarch64_field_kind enum names
> > as macros that expand to corresponding the aarch64_field value.  I've
> > deliberately chosen not to inline all of these remaining uses immediately, as
> > in many cases I intend to either add comments or rework the surrounding code
> > first.  After this patch series, there would be 76 macros remaining (not
> > counting the FLD_CONST_* macros), out of 216 original enum values.
> > 
> > 
> > Does this approach look ok?
> > 
> > Thanks,
> > Alice
> 
> R.