gcc generating calls to __udivdi3

gcc generating calls to __udivdi3

[Ralf Baechle]

Forwarding this to linux-arch mostly as a heads up to other arch maintainers
and discussion.  With gcc's optimizer getting increasingly smarter I'm not
so sure if below patch is the right thing.  In timespec_add_ns we would
normally except the loop to be interated once at most and gcc should make
this assumption due to the unlikely() hint.  Anyway, we have to be
prepared for more such code transformations to happen so adding a __udivdi3
implementation is probably the defensive approach.

  Ralf

From:	Thiemo Seufer <ths@networkno.de>
Date:	Tue, 1 May 2007 19:11:21 +0100
Subject: [PATCH] Use do_div for a subtract loop

Hello All,

this patch switches the subtract loop in timespec_add_ns to use
do_div. The latest GCC SVN version grew intelligent enough to
optimize the loop to a division which calls libgcc's __udivdi3,
which breaks kernel builds.

Tested by building and booting a little endian qemu MIPS kernel.


Thiemo


Signed-off-by: Thiemo Seufer <ths@networkno.de>
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/include/linux/time.h b/include/linux/time.h
index 8ea8dea..e1a11d7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -5,6 +5,7 @@
 
 #ifdef __KERNEL__
 # include <linux/seqlock.h>
+# include <asm/div64.h>
 #endif
 
 #ifndef _STRUCT_TIMESPEC
@@ -169,9 +170,10 @@ extern struct timeval ns_to_timeval(const s64 nsec);
 static inline void timespec_add_ns(struct timespec *a, u64 ns)
 {
 	ns += a->tv_nsec;
-	while(unlikely(ns >= NSEC_PER_SEC)) {
-		ns -= NSEC_PER_SEC;
-		a->tv_sec++;
+	if(unlikely(ns >= NSEC_PER_SEC)) {
+		u64 tmp = ns;
+		ns = do_div(tmp, NSEC_PER_SEC);
+		a->tv_sec += tmp;
 	}
 	a->tv_nsec = ns;
 }

Re: gcc generating calls to __udivdi3

[David Miller]

From: Ralf Baechle <ralf@linux-mips.org>
Date: Thu, 3 May 2007 18:38:42 +0100

> Forwarding this to linux-arch mostly as a heads up to other arch maintainers
> and discussion.  With gcc's optimizer getting increasingly smarter I'm not
> so sure if below patch is the right thing.  In timespec_add_ns we would
> normally except the loop to be interated once at most and gcc should make
> this assumption due to the unlikely() hint.  Anyway, we have to be
> prepared for more such code transformations to happen so adding a __udivdi3
> implementation is probably the defensive approach.

I would rather not have it there and therefore know when such
an expensive operation is being emitted by gcc.

It looks like the divide costs might be set wrong on MIPS
in gcc if it thinks udivdi3 is cheaper than this loop.

Re: gcc generating calls to __udivdi3

[Segher Boessenkool]

>> Forwarding this to linux-arch mostly as a heads up to other arch 
>> maintainers
>> and discussion.  With gcc's optimizer getting increasingly smarter 
>> I'm not
>> so sure if below patch is the right thing.  In timespec_add_ns we 
>> would
>> normally except the loop to be interated once at most and gcc should 
>> make
>> this assumption due to the unlikely() hint.  Anyway, we have to be
>> prepared for more such code transformations to happen so adding a 
>> __udivdi3
>> implementation is probably the defensive approach.
>
> I would rather not have it there and therefore know when such
> an expensive operation is being emitted by gcc.
>
> It looks like the divide costs might be set wrong on MIPS
> in gcc if it thinks udivdi3 is cheaper than this loop.

More likely GCC doesn't know how many times this
loop would run on average, making the loop very
expensive indeed.  Perhaps the loop->div optimisation
just doesn't see the "unlikely()".  How about filing
a bug report?  The original mail says this only
happens with SVN versions of GCC, so if it is handled
fast enough you won't have to deal with this in the
kernel at all.


Segher