* [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
@ 2012-09-01 18:53 Jean Delvare
2012-09-02 4:54 ` Guenter Roeck
2012-09-18 15:49 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 6+ messages in thread
From: Jean Delvare @ 2012-09-01 18:53 UTC (permalink / raw)
To: LMML; +Cc: Andrew Morton, Guenter Roeck, Mauro Carvalho Chehab
DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
dealing with unsigned dividends.
Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
---
drivers/media/rc/mceusb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
+++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
@@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
break;
case MCE_RSP_EQIRCFS:
period = DIV_ROUND_CLOSEST(
- (1 << data1 * 2) * (data2 + 1), 10);
+ (1U << data1 * 2) * (data2 + 1), 10);
if (!period)
break;
carrier = (1000 * 1000) / period;
--
Jean Delvare
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
2012-09-01 18:53 [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call Jean Delvare
@ 2012-09-02 4:54 ` Guenter Roeck
2012-09-18 15:49 ` Mauro Carvalho Chehab
1 sibling, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2012-09-02 4:54 UTC (permalink / raw)
To: Jean Delvare; +Cc: LMML, Andrew Morton, Mauro Carvalho Chehab
On Sat, Sep 01, 2012 at 08:53:57PM +0200, Jean Delvare wrote:
> DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
> dealing with unsigned dividends.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Tested-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/media/rc/mceusb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
> +++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
> @@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
> break;
> case MCE_RSP_EQIRCFS:
> period = DIV_ROUND_CLOSEST(
> - (1 << data1 * 2) * (data2 + 1), 10);
> + (1U << data1 * 2) * (data2 + 1), 10);
> if (!period)
> break;
> carrier = (1000 * 1000) / period;
>
>
> --
> Jean Delvare
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
2012-09-01 18:53 [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call Jean Delvare
2012-09-02 4:54 ` Guenter Roeck
@ 2012-09-18 15:49 ` Mauro Carvalho Chehab
2012-09-18 16:24 ` Guenter Roeck
2012-09-18 18:35 ` Jean Delvare
1 sibling, 2 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2012-09-18 15:49 UTC (permalink / raw)
To: Jean Delvare; +Cc: LMML, Andrew Morton, Guenter Roeck, Mauro Carvalho Chehab
Em 01-09-2012 15:53, Jean Delvare escreveu:
> DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
> dealing with unsigned dividends.
>
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> ---
> drivers/media/rc/mceusb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
> +++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
> @@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
> break;
> case MCE_RSP_EQIRCFS:
> period = DIV_ROUND_CLOSEST(
> - (1 << data1 * 2) * (data2 + 1), 10);
> + (1U << data1 * 2) * (data2 + 1), 10);
> if (!period)
> break;
> carrier = (1000 * 1000) / period;
>
>
Hmm... this generates the following warning with "W=1":
drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
Perhaps it makes sense to use an optimized version for unsigned, or to
change the macro to take the data types into account.
Regards,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
2012-09-18 15:49 ` Mauro Carvalho Chehab
@ 2012-09-18 16:24 ` Guenter Roeck
2012-09-18 18:35 ` Jean Delvare
1 sibling, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2012-09-18 16:24 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Jean Delvare, LMML, Andrew Morton, Mauro Carvalho Chehab
On Tue, Sep 18, 2012 at 12:49:53PM -0300, Mauro Carvalho Chehab wrote:
> Em 01-09-2012 15:53, Jean Delvare escreveu:
> > DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
> > dealing with unsigned dividends.
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> > ---
> > drivers/media/rc/mceusb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
> > +++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
> > @@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
> > break;
> > case MCE_RSP_EQIRCFS:
> > period = DIV_ROUND_CLOSEST(
> > - (1 << data1 * 2) * (data2 + 1), 10);
> > + (1U << data1 * 2) * (data2 + 1), 10);
> > if (!period)
> > break;
> > carrier = (1000 * 1000) / period;
> >
> >
>
> Hmm... this generates the following warning with "W=1":
>
> drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
>
> Perhaps it makes sense to use an optimized version for unsigned, or to
> change the macro to take the data types into account.
>
DIV_ROUND_CLOSEST tests "((typeof(x))-1) >= 0" on purpose, so the compiler
can optimize the signed part of the macro away if the variable type is unsigned.
The test was borrowed from C99 code. Would be great if someone has an idea
how to tell the compiler not to create a warning for this test.
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
2012-09-18 15:49 ` Mauro Carvalho Chehab
2012-09-18 16:24 ` Guenter Roeck
@ 2012-09-18 18:35 ` Jean Delvare
2012-09-19 3:46 ` Guenter Roeck
1 sibling, 1 reply; 6+ messages in thread
From: Jean Delvare @ 2012-09-18 18:35 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: LMML, Andrew Morton, Guenter Roeck, Mauro Carvalho Chehab
Hi Mauro,
On Tue, 18 Sep 2012 12:49:53 -0300, Mauro Carvalho Chehab wrote:
> Em 01-09-2012 15:53, Jean Delvare escreveu:
> > DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
> > dealing with unsigned dividends.
> >
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> > ---
> > drivers/media/rc/mceusb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
> > +++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
> > @@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
> > break;
> > case MCE_RSP_EQIRCFS:
> > period = DIV_ROUND_CLOSEST(
> > - (1 << data1 * 2) * (data2 + 1), 10);
> > + (1U << data1 * 2) * (data2 + 1), 10);
> > if (!period)
> > break;
> > carrier = (1000 * 1000) / period;
>
> Hmm... this generates the following warning with "W=1":
>
> drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
I doubt this is the only warning of that kind. There must be a reason
why -Wextra isn't enabled by default.
> Perhaps it makes sense to use an optimized version for unsigned, or to
> change the macro to take the data types into account.
This was discussed before, but Andrew said he preferred a single macro.
And I agree with him, having two macros would induce a risk of the
wrong one being called.
If you can come up with a variant of DIV_ROUND_CLOSEST which performs
the same and doesn't trigger the warning above, we'll be happy to see
it, but neither Guenter nor myself could come up with one.
--
Jean Delvare
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call
2012-09-18 18:35 ` Jean Delvare
@ 2012-09-19 3:46 ` Guenter Roeck
0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2012-09-19 3:46 UTC (permalink / raw)
To: Jean Delvare
Cc: Mauro Carvalho Chehab, LMML, Andrew Morton, Mauro Carvalho Chehab
On Tue, Sep 18, 2012 at 08:35:09PM +0200, Jean Delvare wrote:
> Hi Mauro,
>
> On Tue, 18 Sep 2012 12:49:53 -0300, Mauro Carvalho Chehab wrote:
> > Em 01-09-2012 15:53, Jean Delvare escreveu:
> > > DIV_ROUND_CLOSEST is faster if the compiler knows it will only be
> > > dealing with unsigned dividends.
> > >
> > > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> > > ---
> > > drivers/media/rc/mceusb.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > --- linux-3.6-rc3.orig/drivers/media/rc/mceusb.c 2012-08-04 21:49:27.000000000 +0200
> > > +++ linux-3.6-rc3/drivers/media/rc/mceusb.c 2012-09-01 18:53:32.053042123 +0200
> > > @@ -627,7 +627,7 @@ static void mceusb_dev_printdata(struct
> > > break;
> > > case MCE_RSP_EQIRCFS:
> > > period = DIV_ROUND_CLOSEST(
> > > - (1 << data1 * 2) * (data2 + 1), 10);
> > > + (1U << data1 * 2) * (data2 + 1), 10);
> > > if (!period)
> > > break;
> > > carrier = (1000 * 1000) / period;
> >
> > Hmm... this generates the following warning with "W=1":
> >
> > drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
> > drivers/media/rc/mceusb.c:629:4: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
>
> I doubt this is the only warning of that kind. There must be a reason
> why -Wextra isn't enabled by default.
>
> > Perhaps it makes sense to use an optimized version for unsigned, or to
> > change the macro to take the data types into account.
>
> This was discussed before, but Andrew said he preferred a single macro.
> And I agree with him, having two macros would induce a risk of the
> wrong one being called.
>
> If you can come up with a variant of DIV_ROUND_CLOSEST which performs
> the same and doesn't trigger the warning above, we'll be happy to see
> it, but neither Guenter nor myself could come up with one.
>
I did some more research, and I think I found a fix. I'll send out a patch
in a minute for people to try.
Guenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-19 3:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-01 18:53 [PATCH] [media] mceusb: Optimize DIV_ROUND_CLOSEST call Jean Delvare
2012-09-02 4:54 ` Guenter Roeck
2012-09-18 15:49 ` Mauro Carvalho Chehab
2012-09-18 16:24 ` Guenter Roeck
2012-09-18 18:35 ` Jean Delvare
2012-09-19 3:46 ` Guenter Roeck
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.