Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

[Andrew Morton]

On Fri, 18 May 2007 12:41:24 -0700
bugme-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=8501

Problem.

gcc-4.3 appears to have cunningly converted this:

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++;
	}
	a->tv_nsec = ns;
}

into a divide-by-1000000000 operation, so it emits a call to udivdi3 and we
don't link.

I expect that this optimisation will remain in gcc-4.3 and we'll end up
having major kernel releases which don't build on i386 with major gcc
releases, which isn't altogether desirable.  I suspect we'll need to fix this
fairly urgently, and to backport the fix into a number of kernel releases.

We use the above idiom in several places.  A suitable fix might be to hunt
down those various sites and then make them call a helper function which
does

	if (unlikely(ns >= NSEC_PER_SEC)) {
		do_div(...)
	}

(Better would be to inline the comparison and to uninline the do_div(),
if it's a 32-bit arch.  Doing all this in a backportable fashion may
prove tricky)

Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

[Adrian Bunk]

On Fri, May 18, 2007 at 03:29:35PM -0700, Andrew Morton wrote:
>...
> I expect that this optimisation will remain in gcc-4.3 and we'll end up
> having major kernel releases which don't build on i386 with major gcc
> releases, which isn't altogether desirable.  I suspect we'll need to fix this
> fairly urgently, and to backport the fix into a number of kernel releases.
>...

Building old kernels with the latest gcc has never worked [1].

First of all, gcc 4.3 is still one year away from being released, so 
there's zero urgency.

And considering that compile errors are the trivial things, and the real 
problems are new runtime errors caused by incorrect kernel code combined 
with gcc optimization [2] I'd even consider backporting only the compile 
fixes before the real issues have been shaken out a bad idea.

cu
Adrian

[1] the backported gcc 4 support in kernel 2.4 is an exception
[2] look e.g. at commit 8690ba446defe2e2b81803756c099d2943dfd5fd

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

[Thomas Gleixner]

On Fri, 2007-05-18 at 15:29 -0700, Andrew Morton wrote:
> We use the above idiom in several places.  A suitable fix might be to hunt
> down those various sites and then make them call a helper function which
> does
> 
> 	if (unlikely(ns >= NSEC_PER_SEC)) {
> 		do_div(...)
> 	}
> 
> (Better would be to inline the comparison and to uninline the do_div(),
> if it's a 32-bit arch.  Doing all this in a backportable fashion may
> prove tricky)

A suitable fix would be to slap the gcc maintainers with a large trout
to fix this nonsense. At least they should provide a command line option
to prevent this extra intelligent optimization.

	tglx

Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

[Linus Torvalds]

On Fri, 18 May 2007, Andrew Morton wrote:
> 
> gcc-4.3 appears to have cunningly converted this:

Very cunning indeed.

Considerign that gcc converted straightforward and simple code to a total 
disaster with a 64-bit divide, I'd call it a gcc bug.

> into a divide-by-1000000000 operation, so it emits a call to udivdi3 and we
> don't link.

I think the proper fix is to just tell people that version of gcc is 
broken.

		Linus

Re: [Bug 8501] udivdi3 absence with gcc-4.3.0 on kernels 2.6.20.11 & 2.6.22.-rc1

[Segher Boessenkool]

> gcc-4.3 appears to have cunningly converted this:
>
> 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++;
> 	}
> 	a->tv_nsec = ns;
> }
>
> into a divide-by-1000000000 operation, so it emits a call to udivdi3 
> and we
> don't link.

Exactly.  It obviously is a bug in the kernel that it depends
on certain compiler optimisations that it doesn't have direct
control over to happen or not.  OTOH, GCC's behaviour here is
probably a non-optimal code issue; it doesn't seem to take the
unlikely() into account when doing the loop transform.

> I expect that this optimisation will remain in gcc-4.3

If someone files a *useable* problem report it most likely
will be taken care of, actually.

> and we'll end up
> having major kernel releases which don't build on i386 with major gcc
> releases, which isn't altogether desirable.

Yeah, like 4.2.0 with powerpc.  Seems like no one tested it :-(

> I suspect we'll need to fix this
> fairly urgently, and to backport the fix into a number of kernel 
> releases.

If it is 4.3 only, you could instead try to work *with* the GCC
people.  It _is_ very fragile code of course, it wouldn't hurt
to replace it with something better.

> We use the above idiom in several places.  A suitable fix might be to 
> hunt
> down those various sites and then make them call a helper function 
> which
> does
>
> 	if (unlikely(ns >= NSEC_PER_SEC)) {
> 		do_div(...)
> 	}
>
> (Better would be to inline the comparison and to uninline the do_div(),
> if it's a 32-bit arch.  Doing all this in a backportable fashion may
> prove tricky)

Perhaps putting a compiler barrier in there would be enough -- like
an empty asm() that takes the loop variable as input.


Segher