2002-10-31 03:10:56

by Nakajima, Jun

[permalink] [raw]
Subject: [PATCH] fixes for building kernel 2.5.45 using Intel compiler

Linus,

Attached is the updated patch that resolves the issues when building the
Linux kernel
with Intel compiler. I reflected the comments from the mail list.
We would like to get this patch incorporated to allow the kernel built with
Intel Compiler.
Hopefully it's not wrapped around this time.

Thanks,
Jun Nakajima

diff -ur linux-2.5.45.orig/arch/i386/kernel/ioport.c
linux-2.5.45/arch/i386/kernel/ioport.c
--- linux-2.5.45.orig/arch/i386/kernel/ioport.c Sun Oct 20 12:30:45 2002
+++ linux-2.5.45/arch/i386/kernel/ioport.c Wed Oct 30 18:24:38 2002
@@ -110,7 +110,7 @@

asmlinkage int sys_iopl(unsigned long unused)
{
- struct pt_regs * regs = (struct pt_regs *) &unused;
+ volatile struct pt_regs * regs = (struct pt_regs *) &unused;
unsigned int level = regs->ebx;
unsigned int old = (regs->eflags >> 12) & 3;

diff -ur linux-2.5.45.orig/drivers/net/wireless/airo.c
linux-2.5.45/drivers/net/wireless/airo.c
--- linux-2.5.45.orig/drivers/net/wireless/airo.c Sun Oct 20 12:30:45
2002
+++ linux-2.5.45/drivers/net/wireless/airo.c Wed Oct 30 18:24:38 2002
@@ -97,12 +97,12 @@
infront of the label, that statistic will not be included in the list
of statistics in the /proc filesystem */

-#define IGNLABEL 0&(int)
+#define IGNLABEL (0,)
static char *statsLabels[] = {
"RxOverrun",
- IGNLABEL "RxPlcpCrcErr",
- IGNLABEL "RxPlcpFormatErr",
- IGNLABEL "RxPlcpLengthErr",
+ IGNLABEL /* "RxPlcpCrcErr", */
+ IGNLABEL /* "RxPlcpFormatErr", */
+ IGNLABEL /* "RxPlcpLengthErr", */
"RxMacCrcErr",
"RxMacCrcOk",
"RxWepErr",
@@ -146,15 +146,15 @@
"HostRxBc",
"HostRxUc",
"HostRxDiscard",
- IGNLABEL "HmacTxMc",
- IGNLABEL "HmacTxBc",
- IGNLABEL "HmacTxUc",
- IGNLABEL "HmacTxFail",
- IGNLABEL "HmacRxMc",
- IGNLABEL "HmacRxBc",
- IGNLABEL "HmacRxUc",
- IGNLABEL "HmacRxDiscard",
- IGNLABEL "HmacRxAccepted",
+ IGNLABEL /* "HmacTxMc", */
+ IGNLABEL /* "HmacTxBc", */
+ IGNLABEL /* "HmacTxUc", */
+ IGNLABEL /* "HmacTxFail", */
+ IGNLABEL /* "HmacRxMc", */
+ IGNLABEL /* "HmacRxBc", */
+ IGNLABEL /* "HmacRxUc", */
+ IGNLABEL /* "HmacRxDiscard", */
+ IGNLABEL /* "HmacRxAccepted", */
"SsidMismatch",
"ApMismatch",
"RatesMismatch",
@@ -162,26 +162,26 @@
"AuthTimeout",
"AssocReject",
"AssocTimeout",
- IGNLABEL "ReasonOutsideTable",
- IGNLABEL "ReasonStatus1",
- IGNLABEL "ReasonStatus2",
- IGNLABEL "ReasonStatus3",
- IGNLABEL "ReasonStatus4",
- IGNLABEL "ReasonStatus5",
- IGNLABEL "ReasonStatus6",
- IGNLABEL "ReasonStatus7",
- IGNLABEL "ReasonStatus8",
- IGNLABEL "ReasonStatus9",
- IGNLABEL "ReasonStatus10",
- IGNLABEL "ReasonStatus11",
- IGNLABEL "ReasonStatus12",
- IGNLABEL "ReasonStatus13",
- IGNLABEL "ReasonStatus14",
- IGNLABEL "ReasonStatus15",
- IGNLABEL "ReasonStatus16",
- IGNLABEL "ReasonStatus17",
- IGNLABEL "ReasonStatus18",
- IGNLABEL "ReasonStatus19",
+ IGNLABEL /* "ReasonOutsideTable", */
+ IGNLABEL /* "ReasonStatus1", */
+ IGNLABEL /* "ReasonStatus2", */
+ IGNLABEL /* "ReasonStatus3", */
+ IGNLABEL /* "ReasonStatus4", */
+ IGNLABEL /* "ReasonStatus5", */
+ IGNLABEL /* "ReasonStatus6", */
+ IGNLABEL /* "ReasonStatus7", */
+ IGNLABEL /* "ReasonStatus8", */
+ IGNLABEL /* "ReasonStatus9", */
+ IGNLABEL /* "ReasonStatus10", */
+ IGNLABEL /* "ReasonStatus11", */
+ IGNLABEL /* "ReasonStatus12", */
+ IGNLABEL /* "ReasonStatus13", */
+ IGNLABEL /* "ReasonStatus14", */
+ IGNLABEL /* "ReasonStatus15", */
+ IGNLABEL /* "ReasonStatus16", */
+ IGNLABEL /* "ReasonStatus17", */
+ IGNLABEL /* "ReasonStatus18", */
+ IGNLABEL /* "ReasonStatus19", */
"RxMan",
"TxMan",
"RxRefresh",
diff -ur linux-2.5.45.orig/include/linux/mtd/compatmac.h
linux-2.5.45/include/linux/mtd/compatmac.h
--- linux-2.5.45.orig/include/linux/mtd/compatmac.h Fri Sep 27 14:50:29
2002
+++ linux-2.5.45/include/linux/mtd/compatmac.h Wed Oct 30 18:24:38 2002
@@ -193,6 +193,7 @@
#define spin_lock_bh(lock) do {start_bh_atomic();spin_lock(lock);} while(0)
#define spin_unlock_bh(lock) do {spin_unlock(lock);end_bh_atomic();}
while(0)
#else
+#include <linux/interrupt.h>
#include <asm/softirq.h>
#include <linux/spinlock.h>
#endif
diff -ur linux-2.5.45.orig/kernel/module.c linux-2.5.45/kernel/module.c
--- linux-2.5.45.orig/kernel/module.c Fri Sep 27 14:49:13 2002
+++ linux-2.5.45/kernel/module.c Wed Oct 30 18:24:38 2002
@@ -425,11 +425,11 @@
printk(KERN_ERR "init_module: mod->deps out of bounds.\n");
goto err2;
}
- if (mod->init && !mod_bound(mod->init, 0, mod)) {
+ if (mod->init && !mod_bound((unsigned long)mod->init, 0, mod)) {
printk(KERN_ERR "init_module: mod->init out of bounds.\n");
goto err2;
}
- if (mod->cleanup && !mod_bound(mod->cleanup, 0, mod)) {
+ if (mod->cleanup && !mod_bound((unsigned long)mod->cleanup, 0, mod))
{
printk(KERN_ERR "init_module: mod->cleanup out of
bounds.\n");
goto err2;
}
@@ -449,7 +449,7 @@
goto err2;
}
if (mod_member_present(mod, can_unload)
- && mod->can_unload && !mod_bound(mod->can_unload, 0, mod)) {
+ && mod->can_unload && !mod_bound((unsigned long)mod->can_unload,
0, mod)) {
printk(KERN_ERR "init_module: mod->can_unload out of
bounds.\n");
goto err2;
}



2002-10-31 10:53:18

by Alan

[permalink] [raw]
Subject: Re: [PATCH] fixes for building kernel 2.5.45 using Intel compiler

On Thu, 2002-10-31 at 03:17, Nakajima, Jun wrote:
> asmlinkage int sys_iopl(unsigned long unused)
> {
> - struct pt_regs * regs = (struct pt_regs *) &unused;
> + volatile struct pt_regs * regs = (struct pt_regs *) &unused;

Why is this needed ?


> - IGNLABEL "HmacRxUc",
> - IGNLABEL "HmacRxDiscard",
> - IGNLABEL "HmacRxAccepted",
> + IGNLABEL /* "HmacTxMc", */
> + IGNLABEL /* "HmacTxBc", */

You seem to be removing fields from the struct - have you tested this ?


2002-10-31 16:32:53

by Nakajima, Jun

[permalink] [raw]
Subject: RE: [PATCH] fixes for building kernel 2.5.45 using Intel compiler

> -----Original Message-----
> From: Alan Cox [mailto:[email protected]]
>
> On Thu, 2002-10-31 at 03:17, Nakajima, Jun wrote:
> > asmlinkage int sys_iopl(unsigned long unused)
> > {
> > - struct pt_regs * regs = (struct pt_regs *) &unused;
> > + volatile struct pt_regs * regs = (struct pt_regs *) &unused;
>
> Why is this needed ?

Becaues the compiler optimization removes the following code without it.
regs->eflags = (regs->eflags & 0xffffcfff) | (level << 12);

The compiler provides access to the argument 'unused' in the stack
(asmlinkage,
i.e. __attribute__ ((regparm(0))), but it thinks modifying the stack
more than that is not effective anyway. So it elimites the code under
optimizations.

>
>
> > - IGNLABEL "HmacRxUc",
> > - IGNLABEL "HmacRxDiscard",
> > - IGNLABEL "HmacRxAccepted",
> > + IGNLABEL /* "HmacTxMc", */
> > + IGNLABEL /* "HmacTxBc", */
>
> You seem to be removing fields from the struct - have you
> tested this ?
>
No, it's not removing fields from there. The original definition of IGNLABLE
is
#define IGNLABEL 0&(int)
And
IGNLABEL "HmacRxUc",
simpile ends up 0, (in gcc). But this is just causing (a lot of) warnings,
so I take this out.

Jun



2002-10-31 18:58:57

by Linus Torvalds

[permalink] [raw]
Subject: RE: [PATCH] fixes for building kernel 2.5.45 using Intel compiler


On 31 Oct 2002, Alan Cox wrote:
>
> The compiler at function entry cannot know anything about the scope of
> objects above the return address. It could equally be a valid pointer to
> data above the stack with a global context created by a thread library.
>
> I'm curious if the optimisation is actually legal

The optimization is not legal or illegal per se, the only thing that can
make it legal or illegal is a defined calling convention. The calling
convention can say who the "owner" of the arguments is: the caller or the
callee.

The kernel doesn't have a well-enough-defined calling convention to be
able to make a good judgement. We tend to use the same calling convention
as the native compiler in user space does, but even that's not always true
(ie it can be modified by things like -mregparm etc, on a per-architecture
basis).

I don't think the original iBCS2 calling convention (that Linux uses on
x86) is specific on this issue. That would be the thing that would decide
the legality of the optimization, I think.

Considering that Intel largely wrote iBCS2, I guess some Intel person can
know what the standard was ;)

Linus

2002-10-31 18:47:17

by Alan

[permalink] [raw]
Subject: RE: [PATCH] fixes for building kernel 2.5.45 using Intel compiler

On Thu, 2002-10-31 at 16:37, Nakajima, Jun wrote:
> > Why is this needed ?
>
> Becaues the compiler optimization removes the following code without it.
> regs->eflags = (regs->eflags & 0xffffcfff) | (level << 12);
>
> The compiler provides access to the argument 'unused' in the stack
> (asmlinkage,
> i.e. __attribute__ ((regparm(0))), but it thinks modifying the stack
> more than that is not effective anyway. So it elimites the code under
> optimizations.

The compiler at function entry cannot know anything about the scope of
objects above the return address. It could equally be a valid pointer to
data above the stack with a global context created by a thread library.

I'm curious if the optimisation is actually legal


> > > - IGNLABEL "HmacRxUc",
> > > - IGNLABEL "HmacRxDiscard",
> > > - IGNLABEL "HmacRxAccepted",
> > > + IGNLABEL /* "HmacTxMc", */
> > > + IGNLABEL /* "HmacTxBc", */
> >
> > You seem to be removing fields from the struct - have you
> > tested this ?
> >
> No, it's not removing fields from there. The original definition of IGNLABLE
> is
> #define IGNLABEL 0&(int)
> And
> IGNLABEL "HmacRxUc",
> simpile ends up 0, (in gcc). But this is just causing (a lot of) warnings,
> so I take this out.

You removed the comma in the patch above its gone from IGNLABEL foo, to
IGNLABEL foo. I don't see where your comma is coming back from.
Otherwise I've no problem (although maybe IGNLABEL("HmacRxAccepted")
would be neater since it conveniently lets people put them back.

Alan