[3/8] pinctrl: renesas: Reorder struct sh_pfc_pin to remove hole

On arm64, pointer size and alignment is 64-bit, hence a 4-byte hole is
present in between the enum_id and name members of the sh_pfc_pin
structure.  Get rid of this hole by sorting the structure's members by
decreasing size.

This saves up to 1.5 KiB per enabled SoC, and reduces the size of a
kernel including support for all R-Car Gen3 SoCs by more than 10 KiB.

This has no size impact on SH and arm32.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/pinctrl/renesas/sh_pfc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On Wed, Oct 28, 2020 at 4:16 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> On arm64, pointer size and alignment is 64-bit, hence a 4-byte hole is
> present in between the enum_id and name members of the sh_pfc_pin
> structure.  Get rid of this hole by sorting the structure's members by
> decreasing size.
>
> This saves up to 1.5 KiB per enabled SoC, and reduces the size of a
> kernel including support for all R-Car Gen3 SoCs by more than 10 KiB.
>
> This has no size impact on SH and arm32.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/pinctrl/renesas/sh_pfc.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h
> index eff1bb872325ef3a..3b390dffacb4910d 100644
> --- a/drivers/pinctrl/renesas/sh_pfc.h
> +++ b/drivers/pinctrl/renesas/sh_pfc.h
> @@ -34,10 +34,10 @@ enum {
>  #define SH_PFC_PIN_CFG_NO_GPIO         (1 << 31)
>
>  struct sh_pfc_pin {
> -       u16 pin;
> -       u16 enum_id;
>         const char *name;
>         unsigned int configs;
> +       u16 pin;
> +       u16 enum_id;
>  };

Hehehe :D

The compiler people have something that is called "premature optimization"
which is when you try to outsmart the compiler.

So since you have metrics on this you have obviously outsmarted the
ARM64 compiler (I guess GCC).

What I'm thinking is that some compiler person should look at this
and say that "yeah sometimes you have to do that". In this case
I suppose the compiler really isn't allowed to reshuffle struct members
in memory since there is plenty of code that relies on them being
laid out strictly in the order they are defined into the struct. So this
is really necessary.

Second I think it warrants a comment in the code to be careful with
aligning structs on 64bit boundaries?

Yours,
Linus Walleij

Hi Linus,

On Thu, Nov 5, 2020 at 10:52 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Oct 28, 2020 at 4:16 PM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> > On arm64, pointer size and alignment is 64-bit, hence a 4-byte hole is
> > present in between the enum_id and name members of the sh_pfc_pin
> > structure.  Get rid of this hole by sorting the structure's members by
> > decreasing size.
> >
> > This saves up to 1.5 KiB per enabled SoC, and reduces the size of a
> > kernel including support for all R-Car Gen3 SoCs by more than 10 KiB.
> >
> > This has no size impact on SH and arm32.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/pinctrl/renesas/sh_pfc.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pinctrl/renesas/sh_pfc.h b/drivers/pinctrl/renesas/sh_pfc.h
> > index eff1bb872325ef3a..3b390dffacb4910d 100644
> > --- a/drivers/pinctrl/renesas/sh_pfc.h
> > +++ b/drivers/pinctrl/renesas/sh_pfc.h
> > @@ -34,10 +34,10 @@ enum {
> >  #define SH_PFC_PIN_CFG_NO_GPIO         (1 << 31)
> >
> >  struct sh_pfc_pin {
> > -       u16 pin;
> > -       u16 enum_id;
> >         const char *name;
> >         unsigned int configs;
> > +       u16 pin;
> > +       u16 enum_id;
> >  };
>
> Hehehe :D
>
> The compiler people have something that is called "premature optimization"
> which is when you try to outsmart the compiler.
>
> So since you have metrics on this you have obviously outsmarted the
> ARM64 compiler (I guess GCC).
>
> What I'm thinking is that some compiler person should look at this
> and say that "yeah sometimes you have to do that". In this case
> I suppose the compiler really isn't allowed to reshuffle struct members
> in memory since there is plenty of code that relies on them being
> laid out strictly in the order they are defined into the struct. So this
> is really necessary.

The compiler is not allowed to reorder the members (FWIW, this
might be a description of hardware register layout).

> Second I think it warrants a comment in the code to be careful with
> aligning structs on 64bit boundaries?

IMHO that's overkill: if we go that route, we have to add such comments
to every structure that contains members of different sizes...

Gr{oetje,eeting}s,

                        Geert