public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB•COM>
To: 'Anson Huang' <anson.huang@nxp•com>,
	"thierry.reding@gmail•com" <thierry.reding@gmail•com>,
	"shawnguo@kernel•org" <shawnguo@kernel•org>,
	"s.hauer@pengutronix•de" <s.hauer@pengutronix•de>,
	"kernel@pengutronix•de" <kernel@pengutronix•de>,
	"festevam@gmail•com" <festevam@gmail•com>,
	"linux-pwm@vger•kernel.org" <linux-pwm@vger•kernel.org>,
	"linux-arm-kernel@lists•infradead.org"
	<linux-arm-kernel@lists•infradead.org>,
	"linux-kernel@vger•kernel.org" <linux-kernel@vger•kernel.org>
Cc: dl-linux-imx <linux-imx@nxp•com>
Subject: RE: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it repeatedly
Date: Tue, 24 Sep 2019 10:43:35 +0000	[thread overview]
Message-ID: <37b2481ac9094f27bf21325e2770abdb@AcuMS.aculab.com> (raw)
In-Reply-To: <DB3PR0402MB3916FFD66797DAC0AB1110D8F5840@DB3PR0402MB3916.eurprd04.prod.outlook.com>

From: Anson Huang
> Sent: 24 September 2019 11:03
> Hi, David
> 
> > Subject: RE: [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it
> > repeatedly
> >
> > From: Anson Huang
> > > Sent: 24 September 2019 10:00
> > > Add helper variable dev = &pdev->dev to simply the code.
> > >
...
> > >  static int pwm_imx27_probe(struct platform_device *pdev)  {
> > > +	struct device *dev = &pdev->dev;
> > >  	struct pwm_imx27_chip *imx;
> > >
> > > -	imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
> > > +	imx = devm_kzalloc(dev, sizeof(*imx), GFP_KERNEL);
...
> > Hopefully the compiler will optimise this back otherwise you've added
> > another local variable which may cause spilling to stack.
> > For a setup function it probably doesn't matter, but in general it might have a
> > small negative performance impact.
> >
> > In any case this doesn't shorten any lines enough to remove line-wrap and
> > using &pdev->dev is really one less variable to mentally track when reading
> > the code.
> 
> Do we know which compiler will optimize this? I saw many of the patches doing
> this to avoid a lot of dereference, I understand it does NOT save lines, but my intention
> is to avoid dereference which might save some instructions.
> 
> I thought saving instructions is more important. So now there are different opinion about
> doing this?

Remember &pdev->dev is just 'pdev + constant'.
Assuming 'pdev' is held in a callee saved register (which you want it to be) then to access
dev->foo the compiler can remember the constant and use an offset from 'pdev' instead of
an extra 'dev' variable.
On most modern ABI the first function call arguments are passed in registers.
So an add  instruction (probably lea) can be used to add the constant offset at the same
time as the value is moved into the argument register.

However your extra variable could easily get spilled out to the stack.
So you get an extra memory read rather than (at most) an extra 'add' instruction.

Even if pdev->dev were a pointer, repeatedly reading it from pdev->dev could
easily generate better code than having an extra variable that would mean the
value was repeatedly read from the stack.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists•infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-09-24 10:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-24  8:59 [PATCH] pwm: pwm-imx27: Use 'dev' instead of dereferencing it repeatedly Anson Huang
2019-09-24  9:46 ` David Laight
2019-09-24 10:03   ` Anson Huang
2019-09-24 10:43     ` David Laight [this message]
2019-09-25  2:36       ` Anson Huang
2019-09-24 10:52   ` Thierry Reding
2019-10-02  7:33   ` Uwe Kleine-König

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=37b2481ac9094f27bf21325e2770abdb@AcuMS.aculab.com \
    --to=david.laight@aculab$(echo .)com \
    --cc=anson.huang@nxp$(echo .)com \
    --cc=festevam@gmail$(echo .)com \
    --cc=kernel@pengutronix$(echo .)de \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-imx@nxp$(echo .)com \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=linux-pwm@vger$(echo .)kernel.org \
    --cc=s.hauer@pengutronix$(echo .)de \
    --cc=shawnguo@kernel$(echo .)org \
    --cc=thierry.reding@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox