2003-09-28 16:29:09

by Brian Gerst

[permalink] [raw]
Subject: [PATCH] i386 do_machine_check() is redundant.

diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.c linux/arch/i386/kernel/cpu/mcheck/mce.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.c 2003-07-27 13:06:29.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/mce.c 2003-09-28 12:23:16.267949368 -0400
@@ -26,11 +26,6 @@
/* Call the installed machine check handler for this CPU setup. */
void (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;

-asmlinkage void do_machine_check(struct pt_regs * regs, long error_code)
-{
- machine_check_vector(regs, error_code);
-}
-
/* This has to be run for each processor */
void __init mcheck_init(struct cpuinfo_x86 *c)
{
diff -urN linux-2.6.0-test6/arch/i386/kernel/entry.S linux/arch/i386/kernel/entry.S
--- linux-2.6.0-test6/arch/i386/kernel/entry.S 2003-09-28 10:20:13.981227536 -0400
+++ linux/arch/i386/kernel/entry.S 2003-09-28 12:23:16.268949216 -0400
@@ -595,7 +595,7 @@
#ifdef CONFIG_X86_MCE
ENTRY(machine_check)
pushl $0
- pushl $do_machine_check
+ pushl machine_check_vector
jmp error_code
#endif


Attachments:
mce-1 (1.05 kB)

2003-09-28 18:24:50

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.


On Sun, 28 Sep 2003, Brian Gerst wrote:
>
> Use machine_check_vector in the entry code instead.

This is wrong. You just lost the "asmlinkage" thing, which means that it
breaks when asmlinkage matters.

And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
one thing, so it makes a huge difference if the kernel is compiled with
-mregparm=3 (which used to work, and which I'd love to do, but gcc has
often been a tad fragile).

Linus

2003-09-28 18:30:05

by Brian Gerst

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.

Linus Torvalds wrote:
> On Sun, 28 Sep 2003, Brian Gerst wrote:
>
>>Use machine_check_vector in the entry code instead.
>
>
> This is wrong. You just lost the "asmlinkage" thing, which means that it
> breaks when asmlinkage matters.
>
> And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> one thing, so it makes a huge difference if the kernel is compiled with
> -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> often been a tad fragile).
>
> Linus
>

Good point. Wouldn't it just be better to change the few handlers to
asmlinkage instead? Having that stub function there is pointless.

--
Brian Gerst

2003-09-28 18:34:11

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.


On Sun, 28 Sep 2003, Brian Gerst wrote:
>
> Good point. Wouldn't it just be better to change the few handlers to
> asmlinkage instead? Having that stub function there is pointless.

That would work, yes. One problem is that gcc doesn't do proper
type-checking on it, so it's open to problems. But I'd accept the patch.

Linus

2003-09-28 18:44:42

by Brian Gerst

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.

diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/k7.c linux/arch/i386/kernel/cpu/mcheck/k7.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/k7.c 2003-09-28 10:20:05.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/k7.c 2003-09-28 14:37:06.308207128 -0400
@@ -17,7 +17,7 @@
#include "mce.h"

/* Machine Check Handler For AMD Athlon/Duron */
-static void k7_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void k7_machine_check(struct pt_regs * regs, long error_code)
{
int recover=1;
u32 alow, ahigh, high, low;
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.c linux/arch/i386/kernel/cpu/mcheck/mce.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.c 2003-07-27 13:06:29.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/mce.c 2003-09-28 14:33:11.331928952 -0400
@@ -18,18 +18,13 @@
int nr_mce_banks;

/* Handle unconfigured int18 (should never happen) */
-static void unexpected_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void unexpected_machine_check(struct pt_regs * regs, long error_code)
{
printk(KERN_ERR "CPU#%d: Unexpected int18 (Machine Check).\n", smp_processor_id());
}

/* Call the installed machine check handler for this CPU setup. */
-void (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;
-
-asmlinkage void do_machine_check(struct pt_regs * regs, long error_code)
-{
- machine_check_vector(regs, error_code);
-}
+void asmlinkage (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;

/* This has to be run for each processor */
void __init mcheck_init(struct cpuinfo_x86 *c)
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.h linux/arch/i386/kernel/cpu/mcheck/mce.h
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/mce.h 2003-07-27 12:57:41.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/mce.h 2003-09-28 14:33:17.421003272 -0400
@@ -7,7 +7,7 @@
void winchip_mcheck_init(struct cpuinfo_x86 *c);

/* Call the installed machine check handler for this CPU setup. */
-extern void (*machine_check_vector)(struct pt_regs *, long error_code);
+extern asmlinkage void (*machine_check_vector)(struct pt_regs *, long error_code);

extern int mce_disabled __initdata;
extern int nr_mce_banks;
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p4.c linux/arch/i386/kernel/cpu/mcheck/p4.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p4.c 2003-07-27 13:04:07.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/p4.c 2003-09-28 14:32:26.717711344 -0400
@@ -148,7 +148,7 @@
return mce_num_extended_msrs;
}

-static void intel_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void intel_machine_check(struct pt_regs * regs, long error_code)
{
int recover=1;
u32 alow, ahigh, high, low;
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p5.c linux/arch/i386/kernel/cpu/mcheck/p5.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p5.c 2003-07-27 13:03:24.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/p5.c 2003-09-28 14:32:33.271714984 -0400
@@ -16,7 +16,7 @@
#include "mce.h"

/* Machine check handler for Pentium class Intel */
-static void pentium_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void pentium_machine_check(struct pt_regs * regs, long error_code)
{
u32 loaddr, hi, lotype;
rdmsr(MSR_IA32_P5_MC_ADDR, loaddr, hi);
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p6.c linux/arch/i386/kernel/cpu/mcheck/p6.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/p6.c 2003-07-27 13:04:11.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/p6.c 2003-09-28 14:32:39.256805112 -0400
@@ -16,7 +16,7 @@
#include "mce.h"

/* Machine Check Handler For PII/PIII */
-static void intel_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void intel_machine_check(struct pt_regs * regs, long error_code)
{
int recover=1;
u32 alow, ahigh, high, low;
diff -urN linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/winchip.c linux/arch/i386/kernel/cpu/mcheck/winchip.c
--- linux-2.6.0-test6/arch/i386/kernel/cpu/mcheck/winchip.c 2003-07-27 13:02:34.000000000 -0400
+++ linux/arch/i386/kernel/cpu/mcheck/winchip.c 2003-09-28 14:32:45.437865448 -0400
@@ -15,7 +15,7 @@
#include "mce.h"

/* Machine check handler for WinChip C6 */
-static void winchip_machine_check(struct pt_regs * regs, long error_code)
+static asmlinkage void winchip_machine_check(struct pt_regs * regs, long error_code)
{
printk(KERN_EMERG "CPU0: Machine Check Exception.\n");
}
diff -urN linux-2.6.0-test6/arch/i386/kernel/entry.S linux/arch/i386/kernel/entry.S
--- linux-2.6.0-test6/arch/i386/kernel/entry.S 2003-09-28 10:20:13.000000000 -0400
+++ linux/arch/i386/kernel/entry.S 2003-09-28 14:30:55.516576024 -0400
@@ -595,7 +595,7 @@
#ifdef CONFIG_X86_MCE
ENTRY(machine_check)
pushl $0
- pushl $do_machine_check
+ pushl machine_check_vector
jmp error_code
#endif


Attachments:
mce-2 (4.87 kB)

2003-09-28 19:04:42

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.

On Sun, 2003-09-28 at 20:24, Linus Torvalds wrote:
> On Sun, 28 Sep 2003, Brian Gerst wrote:
> >
> > Use machine_check_vector in the entry code instead.
>
> This is wrong. You just lost the "asmlinkage" thing, which means that it
> breaks when asmlinkage matters.
>
> And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> one thing, so it makes a huge difference if the kernel is compiled with
> -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> often been a tad fragile).

gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
long standing bug with regparm was fixed and now is believed to work)...
since our makefiles check gcc version already... this can be made gcc
version dependent as well for sure..


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-09-29 18:46:23

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.


On Sun, 28 Sep 2003, Arjan van de Ven wrote:
> >
> > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > one thing, so it makes a huge difference if the kernel is compiled with
> > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > often been a tad fragile).
>
> gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> long standing bug with regparm was fixed and now is believed to work)...
> since our makefiles check gcc version already... this can be made gcc
> version dependent as well for sure..

Has anybody checked out whether the kernel works with -mregparm=3? I
forget who did a lot of the work on it originally, and it certainly _used_
to work fine. The improvements to both code size and performance were, if
I remember correctly, measurable but not huge.

One worry (apart from just broken compilers and missing "asmlinkage"
annotations) is that having compiler-version-dependent calling conventions
makes for another variable to take into account when chasing down bugs and
worrying about things like the Nvidia module etc. So it's probably not
worth doing unless the advantages are clear.

Linus

2003-09-29 19:23:17

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, Sep 29, 2003 at 11:46:12AM -0700, Linus Torvalds wrote:
> Has anybody checked out whether the kernel works with -mregparm=3? I
> forget who did a lot of the work on it originally, and it certainly _used_
> to work fine. The improvements to both code size and performance were, if
> I remember correctly, measurable but not huge.

That jibes with what I would expect (I missed the older threads, but
was nonetheless toying with this idea myself)...

Function arguments on the stack are likely to be in L1 cache anyway,
so accessing arguments is already pretty cheap. And storing the
parameters in registers might increase the number of spills slightly,
for cases, where an argument isn't used much, or at all.

Ideally unit-at-a-time could figure out the optimal -mregparm value :)


> One worry (apart from just broken compilers and missing "asmlinkage"
> annotations) is that having compiler-version-dependent calling conventions
> makes for another variable to take into account when chasing down bugs and
> worrying about things like the Nvidia module etc. So it's probably not
> worth doing unless the advantages are clear.

Well... even with completely open source, you're never gonna have a
working system with modules built using compiler versions and options
that differ from the main kernel image. In the past, changing compiler
versions would definitely affect the module interfaces adversely.

So while I agree with your overall conservatism, worrying about
supporting miscompiled modules is the road to Hades...

Jeff



2003-09-29 19:54:54

by Valdis Klētnieks

[permalink] [raw]
Subject: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, 29 Sep 2003 11:46:12 PDT, Linus Torvalds said:

> Has anybody checked out whether the kernel works with -mregparm=3? I
> forget who did a lot of the work on it originally, and it certainly _used_
> to work fine. The improvements to both code size and performance were, if
> I remember correctly, measurable but not huge.

It works well enough that my laptop made it to initlevel 3. The code size
wasn't drastically smaller (perhaps another 2-3% but I already compile with
-Os). The system "felt" snappier, but I have no benchmark numbers to give. I
could probably get some numbers if anybody cares, but there's a showstopper for
me - I can't make it to initlevel 5 easily:

> One worry (apart from just broken compilers and missing "asmlinkage"
> annotations) is that having compiler-version-dependent calling conventions
> makes for another variable to take into account when chasing down bugs and
> worrying about things like the Nvidia module etc. So it's probably not
> worth doing unless the advantages are clear.

Quite correct - even after recompiling the sourced portions of the NVidia
driver, it dies a horrid death on 'insmod' when the closed-source portion
passes a parameter on the stack and the open side expects the value in a
register, and follows the register value to a quick death....

Yes, yes, I know, I could re-try with the open-source nv driver instead of the
closed-source NVidia driver - but the open-source one costs me more in
performance (as it's unaccelerated) than the mregparm=3 is likely to get me
back....


Attachments:
(No filename) (226.00 B)

2003-09-29 20:20:48

by Mikulas Patocka

[permalink] [raw]
Subject: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

> > > Use machine_check_vector in the entry code instead.
> >
> > This is wrong. You just lost the "asmlinkage" thing, which means that it
> > breaks when asmlinkage matters.
> >
> > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > one thing, so it makes a huge difference if the kernel is compiled with
> > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > often been a tad fragile).
>
> gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> long standing bug with regparm was fixed and now is believed to work)...
> since our makefiles check gcc version already... this can be made gcc
> version dependent as well for sure..

They are still buggy. gcc 3.3.1 miscompiles itself with -mregparm=3
(without -O or -O2 it works). (I am too lazy to spend several days trying
to find exactly which function in gcc was miscompiled, maybe I do it one
day). gcc 2.95.3 compiles gcc 3.3.1 with -mregparm=3 -O2 correctly.
gcc 3.4 doesn't seem to be better.

gcc 2.7.2.3 has totally broken -mregparm=3, even quite simple programs
fail.

Mikulas

2003-09-29 20:26:15

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, Sep 29, 2003 at 10:20:45PM +0200, Mikulas Patocka wrote:
> > > > Use machine_check_vector in the entry code instead.
> > >
> > > This is wrong. You just lost the "asmlinkage" thing, which means that it
> > > breaks when asmlinkage matters.
> > >
> > > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > > one thing, so it makes a huge difference if the kernel is compiled with
> > > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > > often been a tad fragile).
> >
> > gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> > long standing bug with regparm was fixed and now is believed to work)...
> > since our makefiles check gcc version already... this can be made gcc
> > version dependent as well for sure..
>
> They are still buggy. gcc 3.3.1 miscompiles itself with -mregparm=3
> (without -O or -O2 it works). (I am too lazy to spend several days trying
> to find exactly which function in gcc was miscompiled, maybe I do it one
> day). gcc 2.95.3 compiles gcc 3.3.1 with -mregparm=3 -O2 correctly.
> gcc 3.4 doesn't seem to be better.
>
> gcc 2.7.2.3 has totally broken -mregparm=3, even quite simple programs
> fail.

You can't build GCC with -mregparm=3. It changes the interface to
system functions. So unless your libc happened to be built with
-mregparm=3, and extensively hacked to expect arguments in registers to
the assembly stubs, it can't work.

It's interesting for kernel code, whole distributions, or things which
are careful to have a glue layer.

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer

2003-09-29 21:36:51

by Mikulas Patocka

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.



On Mon, 29 Sep 2003, Daniel Jacobowitz wrote:

> On Mon, Sep 29, 2003 at 10:20:45PM +0200, Mikulas Patocka wrote:
> > > > > Use machine_check_vector in the entry code instead.
> > > >
> > > > This is wrong. You just lost the "asmlinkage" thing, which means that it
> > > > breaks when asmlinkage matters.
> > > >
> > > > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > > > one thing, so it makes a huge difference if the kernel is compiled with
> > > > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > > > often been a tad fragile).
> > >
> > > gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> > > long standing bug with regparm was fixed and now is believed to work)...
> > > since our makefiles check gcc version already... this can be made gcc
> > > version dependent as well for sure..
> >
> > They are still buggy. gcc 3.3.1 miscompiles itself with -mregparm=3
> > (without -O or -O2 it works). (I am too lazy to spend several days trying
> > to find exactly which function in gcc was miscompiled, maybe I do it one
> > day). gcc 2.95.3 compiles gcc 3.3.1 with -mregparm=3 -O2 correctly.
> > gcc 3.4 doesn't seem to be better.
> >
> > gcc 2.7.2.3 has totally broken -mregparm=3, even quite simple programs
> > fail.
>
> You can't build GCC with -mregparm=3. It changes the interface to
> system functions. So unless your libc happened to be built with
> -mregparm=3, and extensively hacked to expect arguments in registers to
> the assembly stubs, it can't work.

Of course I linked it with libc compiled with regparm=3.

> It's interesting for kernel code, whole distributions, or things which
> are careful to have a glue layer.

BTW. libc headers surround all function parameters with __P, like
extern int printf __P ((__const char* __format, ...));

so you can change in include/sys/cdefs.h
#define __P(x) x
into
#define __P(x) x __attribute__((regparm(0)))

and compile programs with -mregparm=3 -ffreestanding even on normal linux
distribution. I didn't try it for larger program, for simple it works. (it
works as long as program doesn't call libc function via pointer to
function).

(without -ffreestanding gcc sometimes emits calls to library functions on
its own, and uses calling convention from command line, not convention
from prototype).

Mikulas

2003-09-29 21:49:05

by Jakub Jelinek

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, Sep 29, 2003 at 11:36:06PM +0200, Mikulas Patocka wrote:
> > It's interesting for kernel code, whole distributions, or things which
> > are careful to have a glue layer.
>
> BTW. libc headers surround all function parameters with __P, like
> extern int printf __P ((__const char* __format, ...));

s/surround/used to &/

Jakub

2003-09-29 21:43:40

by Mikulas Patocka

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, 29 Sep 2003, Daniel Jacobowitz wrote:

> On Mon, Sep 29, 2003 at 11:36:06PM +0200, Mikulas Patocka wrote:
> >
> >
> > On Mon, 29 Sep 2003, Daniel Jacobowitz wrote:
> >
> > > On Mon, Sep 29, 2003 at 10:20:45PM +0200, Mikulas Patocka wrote:
> > > > > > > Use machine_check_vector in the entry code instead.
> > > > > >
> > > > > > This is wrong. You just lost the "asmlinkage" thing, which means that it
> > > > > > breaks when asmlinkage matters.
> > > > > >
> > > > > > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > > > > > one thing, so it makes a huge difference if the kernel is compiled with
> > > > > > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > > > > > often been a tad fragile).
> > > > >
> > > > > gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> > > > > long standing bug with regparm was fixed and now is believed to work)...
> > > > > since our makefiles check gcc version already... this can be made gcc
> > > > > version dependent as well for sure..
> > > >
> > > > They are still buggy. gcc 3.3.1 miscompiles itself with -mregparm=3
> > > > (without -O or -O2 it works). (I am too lazy to spend several days trying
> > > > to find exactly which function in gcc was miscompiled, maybe I do it one
> > > > day). gcc 2.95.3 compiles gcc 3.3.1 with -mregparm=3 -O2 correctly.
> > > > gcc 3.4 doesn't seem to be better.
> > > >
> > > > gcc 2.7.2.3 has totally broken -mregparm=3, even quite simple programs
> > > > fail.
> > >
> > > You can't build GCC with -mregparm=3. It changes the interface to
> > > system functions. So unless your libc happened to be built with
> > > -mregparm=3, and extensively hacked to expect arguments in registers to
> > > the assembly stubs, it can't work.
> >
> > Of course I linked it with libc compiled with regparm=3.
>
> Did you also hack all the syscall wrappers and hand-coded assembly
> routines? For instance, mmap won't work.

It's not linux glibc, it's freebsd libc. And many things don't work there
yet, that's why I don't release it.

Mikulas

2003-09-29 21:40:39

by Daniel Jacobowitz

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, Sep 29, 2003 at 11:36:06PM +0200, Mikulas Patocka wrote:
>
>
> On Mon, 29 Sep 2003, Daniel Jacobowitz wrote:
>
> > On Mon, Sep 29, 2003 at 10:20:45PM +0200, Mikulas Patocka wrote:
> > > > > > Use machine_check_vector in the entry code instead.
> > > > >
> > > > > This is wrong. You just lost the "asmlinkage" thing, which means that it
> > > > > breaks when asmlinkage matters.
> > > > >
> > > > > And yes, asmlinkage _can_ matter, even on x86. It disasbles regparm, for
> > > > > one thing, so it makes a huge difference if the kernel is compiled with
> > > > > -mregparm=3 (which used to work, and which I'd love to do, but gcc has
> > > > > often been a tad fragile).
> > > >
> > > > gcc 3.2 and later are supposed to be ok (eg during 3.2 development a
> > > > long standing bug with regparm was fixed and now is believed to work)...
> > > > since our makefiles check gcc version already... this can be made gcc
> > > > version dependent as well for sure..
> > >
> > > They are still buggy. gcc 3.3.1 miscompiles itself with -mregparm=3
> > > (without -O or -O2 it works). (I am too lazy to spend several days trying
> > > to find exactly which function in gcc was miscompiled, maybe I do it one
> > > day). gcc 2.95.3 compiles gcc 3.3.1 with -mregparm=3 -O2 correctly.
> > > gcc 3.4 doesn't seem to be better.
> > >
> > > gcc 2.7.2.3 has totally broken -mregparm=3, even quite simple programs
> > > fail.
> >
> > You can't build GCC with -mregparm=3. It changes the interface to
> > system functions. So unless your libc happened to be built with
> > -mregparm=3, and extensively hacked to expect arguments in registers to
> > the assembly stubs, it can't work.
>
> Of course I linked it with libc compiled with regparm=3.

Did you also hack all the syscall wrappers and hand-coded assembly
routines? For instance, mmap won't work.

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer

2003-09-30 00:25:20

by Jamie Lokier

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

Folks compiling the kernel with -mregparm=3 should also try
-mregparm=2, -mregparm=1 and combinations with -mrtd if you want to
find the smallest/fastest.

A long time ago, for a game written mostly in C++, I found the sweet
spot at -mregparm=1 -mrtd.

Enjoy,
-- Jamie

2003-09-30 04:55:11

by Robert Love

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Tue, 2003-09-30 at 00:49, [email protected] wrote:

> I discovered that -test6-mm1 doesn't build with -ffreestanding with gcc 3.3.1,
> for an odd reason: when I specify -ffreestanding, it generates 'call abs' calls
> where it was able to do it inline otherwise. -ffreestanding says there's no library,
> so it can't inline the library call (which leaves no external call to 'abs()').

Hm, we may need to do something like:

#define abs(n) __builtin_abs((n))

because -ffreestanding implies -fno-builtin, which disables use of
built-in functions that do not begin with __builtin.

Robert Love


2003-09-30 04:49:41

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Mon, 29 Sep 2003 23:36:06 +0200, Mikulas Patocka said:

> and compile programs with -mregparm=3 -ffreestanding even on normal linux
> distribution. I didn't try it for larger program, for simple it works. (it
> works as long as program doesn't call libc function via pointer to
> function).

I discovered that -test6-mm1 doesn't build with -ffreestanding with gcc 3.3.1,
for an odd reason: when I specify -ffreestanding, it generates 'call abs' calls
where it was able to do it inline otherwise. -ffreestanding says there's no library,
so it can't inline the library call (which leaves no external call to 'abs()').



Attachments:
(No filename) (226.00 B)

2003-09-30 08:19:51

by Helge Hafting

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

[email protected] wrote:

> Quite correct - even after recompiling the sourced portions of the NVidia
> driver, it dies a horrid death on 'insmod' when the closed-source portion
> passes a parameter on the stack and the open side expects the value in a
> register, and follows the register value to a quick death....
>
Nvidia can fix this easily. Either by having several versions of
their closed-source thing, or by having a open "interface" that
uses nvidia's calling convention for talking to their proprietary
binary code, and whatever the kernel uses for talking to the kernel.
That's the price of binary modules -
One extra level of indirection, or a bunch of different modules.
Of course there are plenty of other cards without
such problems.

Helge Hafting

2003-09-30 14:37:46

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Tue, 30 Sep 2003 00:55:13 EDT, Robert Love said:

> Hm, we may need to do something like:
>
> #define abs(n) __builtin_abs((n))
>
> because -ffreestanding implies -fno-builtin, which disables use of
> built-in functions that do not begin with __builtin.

Well, abs() is the only one I tripped over in my config. I'm sure there's others
lurking elsewhere in the kernel tree.

The bigger question is whether a patch to support -ffreestanding would be a
good idea - with proper use of the __builtin_* stuff it *should* work, and it will
hopefully cause better kernel code hygiene..


Attachments:
(No filename) (226.00 B)

2003-09-30 15:29:38

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Tue, 30 Sep 2003 10:28:40 +0200, Helge Hafting said:

> Nvidia can fix this easily. Either by having several versions of
> their closed-source thing, or by having a open "interface" that
> uses nvidia's calling convention for talking to their proprietary
> binary code, and whatever the kernel uses for talking to the kernel.

Well, they do have an open interface. I've apparently just not gotten all the
__attribute((regparm(0))) in the right places yet. ;)


Attachments:
(No filename) (226.00 B)

2003-09-30 15:48:49

by Robert Love

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.

On Tue, 2003-09-30 at 10:37, [email protected] wrote:

> Well, abs() is the only one I tripped over in my config. I'm sure there's others
> lurking elsewhere in the kernel tree.

There are not _too_ many builtins. abs, strcpy, et cetera ...

> The bigger question is whether a patch to support -ffreestanding would be a
> good idea - with proper use of the __builtin_* stuff it *should* work, and it will
> hopefully cause better kernel code hygiene..

I agree. We should go for it.

Robert Love


2003-09-30 15:48:20

by Linus Torvalds

[permalink] [raw]
Subject: Re: -mregparm=3 (was Re: [PATCH] i386 do_machine_check() is redundant.


On Tue, 30 Sep 2003 [email protected] wrote:
>
> Well, they do have an open interface. I've apparently just not gotten all the
> __attribute((regparm(0))) in the right places yet. ;)

They may have some places inside the binary part that call directly out
to the kernel, and use the wrong calling conventions.

Linus