[PATCH] i386 do_machine_check() is redundant.

[PATCH] i386 do_machine_check() is redundant.

[Brian Gerst]

[-- Attachment #1: Type: text/plain, Size: 72 bytes --]

Use machine_check_vector in the entry code instead.

--
				Brian Gerst

[-- Attachment #2: mce-1 --]
[-- Type: text/plain, Size: 1072 bytes --]

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
 

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

[Linus Torvalds]

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

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

[Brian Gerst]

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

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

[Linus Torvalds]

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

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

[Brian Gerst]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

Linus Torvalds wrote:
> 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
> 

Updated patch.

--
				Brian Gerst

[-- Attachment #2: mce-2 --]
[-- Type: text/plain, Size: 4983 bytes --]

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
 

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

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

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..

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

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

[Linus Torvalds]

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

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

[Jeff Garzik]

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

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1556 bytes --]

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....


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Mikulas Patocka]

> > > 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

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

[Daniel Jacobowitz]

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

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

[Mikulas Patocka]

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

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

[Daniel Jacobowitz]

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

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

[Mikulas Patocka]

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

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

[Jakub Jelinek]

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

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

[Jamie Lokier]

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

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

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()').



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Robert Love]

On Tue, 2003-09-30 at 00:49, Valdis.Kletnieks@vt.edu 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

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

[Helge Hafting]

Valdis.Kletnieks@vt.edu 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

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

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..

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 464 bytes --]

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. ;)


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Linus Torvalds]

On Tue, 30 Sep 2003 Valdis.Kletnieks@vt.edu 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

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

[Robert Love]

On Tue, 2003-09-30 at 10:37, Valdis.Kletnieks@vt.edu 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