Re: [PATCH v3 26/60] target/arm: Replace sentinels with ARRAY_SIZE in cpregs.h

[Richard Henderson]

On 4/22/22 08:36, Alex Bennée wrote:
> > > > +#define define_arm_cp_regs_with_opaque(CPU, REGS, OPAQUE)               \
> > > > +    do {                                                                \
> > > > +        QEMU_BUILD_BUG_ON(ARRAY_SIZE(REGS) == 0);                       \
> > > > +        if (ARRAY_SIZE(REGS) == 1) {                                    \
> > > > +            define_one_arm_cp_reg_with_opaque(CPU, REGS, OPAQUE);       \
> > > > +        } else {                                                        \
> > > > +            define_arm_cp_regs_with_opaque_len(CPU, REGS, OPAQUE,       \
> > > > +                                               ARRAY_SIZE(REGS));       \
> > > > +        }                                                               \
> > > > +    } while (0)
> > > Do we actually need to special case "array has one element" here,
> > > or is this just efficiency?
> > > Anyway
> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > Just efficiency.  There seem to be a lot of these.
>
> If you moved define_arm_cp_regs_with_opaque_len into the header as an
> inline surely the compiler could figure it out when presented with a
> constant i? The would avoid the need for the special casing in the macro
> expansion right?

I didn't want to expand the code size with so many loops.
Anyway, I'm dropping the special case entirely for v4.


r~