All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	Ulrich Hecht <uli+renesas@fpond.eu>,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [PATCH 3/8] pinctrl: renesas: Reorder struct sh_pfc_pin to remove hole
Date: Thu, 5 Nov 2020 10:52:42 +0100	[thread overview]
Message-ID: <CACRpkdYVT0KL4+KdE0QP7UEKCrAUOOS0aAXV7xfY_28DtpvokA@mail.gmail.com> (raw)
In-Reply-To: <20201028151637.1734130-4-geert+renesas@glider.be>

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

  reply	other threads:[~2020-11-05  9:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 15:16 [PATCH 0/8] pinctrl: renesas: Cleanups and improvements Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 1/8] pinctrl: renesas: Remove superfluous goto in sh_pfc_gpio_set_direction() Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 2/8] pinctrl: renesas: Singular/plural grammar fixes Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 3/8] pinctrl: renesas: Reorder struct sh_pfc_pin to remove hole Geert Uytterhoeven
2020-11-05  9:52   ` Linus Walleij [this message]
2020-11-05  9:58     ` Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 4/8] pinctrl: renesas: Optimize sh_pfc_pin_config Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 5/8] pinctrl: renesas: Factor out common R-Car Gen3 bias handling Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 6/8] pinctrl: renesas: r8a7778: Use physical addresses for PUPR regs Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 7/8] pinctrl: renesas: r8a7778: Use common R-Car bias handling Geert Uytterhoeven
2020-10-28 15:16 ` [PATCH 8/8] pinctrl: renesas: Protect GPIO leftovers by CONFIG_PINCTRL_SH_FUNC_GPIO Geert Uytterhoeven
2020-11-05  9:54 ` [PATCH 0/8] pinctrl: renesas: Cleanups and improvements Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CACRpkdYVT0KL4+KdE0QP7UEKCrAUOOS0aAXV7xfY_28DtpvokA@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=geert+renesas@glider.be \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=uli+renesas@fpond.eu \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.