[PATCH] fixes for building kernel using Intel compiler

[PATCH] fixes for building kernel using Intel compiler

[Nakajima, Jun]

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

Hi Linus,

Attached is the patch that resolves some of the redundant code and casting
that are required to build the Linux kernel usning Intel compiler. We would
like to get this patch incorporated to allow the kernel built with Intel
Compiler.

Thanks,
Jun Nakajima

============================================================================
=================diff -ur linux-2.5.43.orig/arch/i386/kernel/ioport.c
linux-2.5.43/arch/i386/kern
el/ioport.c
--- linux-2.5.43.orig/arch/i386/kernel/ioport.c Fri Oct 18 15:19:35 2002
+++ linux-2.5.43/arch/i386/kernel/ioport.c      Fri Oct 18 16:10:13 2002
@@ -108,9 +108,8 @@
  * code.
  */

-asmlinkage int sys_iopl(unsigned long unused)
+asmlinkage int sys_iopl(struct pt_regs * regs)
 {
-       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.43.orig/drivers/net/wireless/airo.c
linux-2.5.43/drivers/net/
wireless/airo.c
--- linux-2.5.43.orig/drivers/net/wireless/airo.c       Fri Oct 18 15:19:35
2002
+++ linux-2.5.43/drivers/net/wireless/airo.c    Fri Oct 18 15:28:06 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.43.orig/include/asm-i386/bugs.h
linux-2.5.43/include/asm-i386
/bugs.h
--- linux-2.5.43.orig/include/asm-i386/bugs.h   Fri Sep 27 14:49:09 2002
+++ linux-2.5.43/include/asm-i386/bugs.h        Fri Oct 18 16:11:56 2002
@@ -76,14 +76,6 @@
                return;
        }

-/* Enable FXSR and company _before_ testing for FP problems. */
-       /*
-        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
-        */
-       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
-               extern void __buggy_fxsr_alignment(void);
-               __buggy_fxsr_alignment();
-       }
        if (cpu_has_fxsr) {
                printk(KERN_INFO "Enabling fast FPU save and restore... ");
                set_in_cr4(X86_CR4_OSFXSR);
diff -ur linux-2.5.43.orig/include/linux/mtd/compatmac.h
linux-2.5.43/include/li
nux/mtd/compatmac.h
--- linux-2.5.43.orig/include/linux/mtd/compatmac.h     Fri Sep 27 14:50:29
2002
+++ linux-2.5.43/include/linux/mtd/compatmac.h  Fri Oct 18 15:28:06 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.43.orig/kernel/module.c linux-2.5.43/kernel/module.c
--- linux-2.5.43.orig/kernel/module.c   Fri Sep 27 14:49:13 2002
+++ linux-2.5.43/kernel/module.c        Fri Oct 18 15:28:06 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;
        }



[-- Attachment #2: icc-build-2.5.43.ZIP --]
[-- Type: application/octet-stream, Size: 1644 bytes --]

Re: [PATCH] fixes for building kernel using Intel compiler

[Greg KH]

On Fri, Oct 18, 2002 at 04:48:34PM -0700, Nakajima, Jun wrote:
> Hi Linus,
> 
> Attached is the patch that resolves some of the redundant code and casting
> that are required to build the Linux kernel usning Intel compiler. We would
> like to get this patch incorporated to allow the kernel built with Intel
> Compiler.

The patch is wrapped, and can't apply.

And what version of the Intel compiler is this for?  I tried to get a
previous version to build a kernel, but I had to change a lot more
things than you did (like build flags, etc.)

thanks,

greg k-h

Re: [PATCH] fixes for building kernel using Intel compiler

[Alan Cox]

On Sat, 2002-10-19 at 00:48, Nakajima, Jun wrote:
> -/* Enable FXSR and company _before_ testing for FP problems. */
> -       /*
> -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
> -        */
> -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
> -               extern void __buggy_fxsr_alignment(void);
> -               __buggy_fxsr_alignment();
> -       }
>         if (cpu_has_fxsr) {
>                 printk(KERN_INFO "Enabling fast FPU save and restore... ");

So you back out a test that is pretty much essential to catch misaligned
stuff if we do get something wrong in our alignments or due to compiler
suprises and hope it doesnt happen ?

This isnt "fixing" this is the mad axeman at work. Linus this patch
should not go in as it is

RE: [PATCH] fixes for building kernel using Intel compiler

[Nakajima, Jun]

I think it depends on the assumptions for the compiler quality. If you don't
trust __attribute__ ((align(xxx)), many other things are broken as well. Why
do you need to check this particular one, especially? For example, even if
__attribute__ ((align(16)) works, __attribute__ ((align(32)) may not work.
But I agree that having such a sanity test is a good thing, and I won't back
it out.

Having said that, one occasion where people might be surprised by gcc (this
might be a known issue, though) is: typedef + __attribute__; it ignores
__attribute__. For example, 
#include <stdio.h>

struct foo_16 {
        char    xxx[3];
        short   yyy;
} __attribute__ ((aligned (16)));

typedef struct bar_16 {
        char    xxx[3];
        short   yyy;
} bar_16_t __attribute__ ((aligned (16)));

struct foo_16   f;
bar_16_t        b0;
bar_16_t        b1;

main() {
        printf("&f = 0x%lx, &b0 = 0x%lx, &b1 = 0x%lx\n", &f, &b0, &b1);
}

$./a.out
&f = 0x8049630, &b0 = 0x8049646, &b1 = 0x8049640

Another example is packed.

#include <stdio.h>
#ifdef TYPEDEF
typedef struct NewSectorMap {
        char byte;
        int c;
} SectorCount __attribute__((packed));
#else
struct SectorCount {
        char byte;
        int c;
}  __attribute__((packed));
#endif


int main() {

#ifdef TYPEDEF
        printf("%p %p\n", &(((SectorCount *)0)->byte), &(((SectorCount
*)0)->c));
#else
        printf("%p %p\n", &(((struct SectorCount *)0)->byte), &(((struct
SectorCount *)0)->c));
#endif
        return 0;
}

$ gcc  typedef.c
$ ./a.out 
(nil) 0x1
$ gcc -D TYPEDEF typedef.c
$ ./a.out 
(nil) 0x4

In the kernel, there are several device drivers (ftape-bsm.h, e100.h, for
example) are doing this kind of thing (i.e. typedef + attribute). 

Thanks,
Jun

-----Original Message-----
From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk]
Sent: Monday, October 21, 2002 5:48 AM
To: Nakajima, Jun
Cc: Linus Torvalds; Linux Kernel Mailing List; Mallick, Asit K; Saxena,
Sunil
Subject: Re: [PATCH] fixes for building kernel using Intel compiler


On Sat, 2002-10-19 at 00:48, Nakajima, Jun wrote:
> -/* Enable FXSR and company _before_ testing for FP problems. */
> -       /*
> -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
> -        */
> -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
> -               extern void __buggy_fxsr_alignment(void);
> -               __buggy_fxsr_alignment();
> -       }
>         if (cpu_has_fxsr) {
>                 printk(KERN_INFO "Enabling fast FPU save and restore...
");

So you back out a test that is pretty much essential to catch misaligned
stuff if we do get something wrong in our alignments or due to compiler
suprises and hope it doesnt happen ?

This isnt "fixing" this is the mad axeman at work. Linus this patch
should not go in as it is

Re: [PATCH] fixes for building kernel using Intel compiler

[Brian Gerst]

Nakajima, Jun wrote:
> I think it depends on the assumptions for the compiler quality. If you don't
> trust __attribute__ ((align(xxx)), many other things are broken as well. Why
> do you need to check this particular one, especially?

Because the hardware requires it.

--
				Brain Gerst

Re: [PATCH] fixes for building kernel using Intel compiler

[Greg KH]

On Mon, Oct 21, 2002 at 08:56:49AM -0700, Nakajima, Jun wrote:
> 
> In the kernel, there are several device drivers (ftape-bsm.h, e100.h, for
> example) are doing this kind of thing (i.e. typedef + attribute). 

All the more reason to not use typedefs in the kernel :)

(sorry, I could not help myself...)

greg k-h

Re: [PATCH] fixes for building kernel using Intel compiler

[Andi Kleen]

"Nakajima, Jun" <jun.nakajima@intel.com> writes:


> -asmlinkage int sys_iopl(unsigned long unused)
> +asmlinkage int sys_iopl(struct pt_regs * regs)
>  {
> -       struct pt_regs * regs = (struct pt_regs *) &unused;

The change is wrong. The pt_regs are passed by value on the stack, not by reference.

> 
> -/* Enable FXSR and company _before_ testing for FP problems. */
> -       /*
> -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
> -        */
> -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
> -               extern void __buggy_fxsr_alignment(void);
> -               __buggy_fxsr_alignment();
> -       }

Why does that not work? IMHO it is legal ISO-C

-Andi

Re: [PATCH] fixes for building kernel using Intel compiler

[David S. Miller]

   From: Andi Kleen <ak@suse.de>
   Date: 19 Oct 2002 02:07:41 +0200
   
   > -/* Enable FXSR and company _before_ testing for FP problems. */
   > -       /*
   > -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
   > -        */
   > -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
   > -               extern void __buggy_fxsr_alignment(void);
   > -               __buggy_fxsr_alignment();
   > -       }
   
   Why does that not work? IMHO it is legal ISO-C

Depending upon the compiler to optimize away the non-existent function
reference is not ISO-C :-)  Although the fact the Intel compiler isn't
doing this is amusing.

Re: [PATCH] fixes for building kernel using Intel compiler

[Andi Kleen]

On Fri, Oct 18, 2002 at 05:15:04PM -0700, David S. Miller wrote:
>    From: Andi Kleen <ak@suse.de>
>    Date: 19 Oct 2002 02:07:41 +0200
>    
>    > -/* Enable FXSR and company _before_ testing for FP problems. */
>    > -       /*
>    > -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte aligned.
>    > -        */
>    > -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
>    > -               extern void __buggy_fxsr_alignment(void);
>    > -               __buggy_fxsr_alignment();
>    > -       }
>    
>    Why does that not work? IMHO it is legal ISO-C
> 
> Depending upon the compiler to optimize away the non-existent function
> reference is not ISO-C :-)  Although the fact the Intel compiler isn't
> doing this is amusing.

True :-)

Well it should. There are tons of similar patterns all over the kernel
which use the same trick.
If it didn't optimize  all of these, there would be many more problems.

-Andi

RE: [PATCH] fixes for building kernel using Intel compiler

[Nakajima, Jun]

No, it removes most of such cases. It happens only for a general boolean
controlling expression, and this is the only spot as far as we tested. But
our argument is that the checking code is not required because
thread.i387.fxsave is __attribute__ ((aligned (16))). If __attribute__
((aligned (...))) is broken, we should see more problems.

Thanks,
Jun

-----Original Message-----
From: Andi Kleen [mailto:ak@suse.de]
Sent: Friday, October 18, 2002 5:25 PM
To: David S. Miller
Cc: ak@suse.de; jun.nakajima@intel.com; torvalds@transmeta.com;
linux-kernel@vger.kernel.org; asit.k.mallick@intel.com;
sunil.saxena@intel.com
Subject: Re: [PATCH] fixes for building kernel using Intel compiler


On Fri, Oct 18, 2002 at 05:15:04PM -0700, David S. Miller wrote:
>    From: Andi Kleen <ak@suse.de>
>    Date: 19 Oct 2002 02:07:41 +0200
>    
>    > -/* Enable FXSR and company _before_ testing for FP problems. */
>    > -       /*
>    > -        * Verify that the FXSAVE/FXRSTOR data will be 16-byte
aligned.
>    > -        */
>    > -       if (offsetof(struct task_struct, thread.i387.fxsave) & 15) {
>    > -               extern void __buggy_fxsr_alignment(void);
>    > -               __buggy_fxsr_alignment();
>    > -       }
>    
>    Why does that not work? IMHO it is legal ISO-C
> 
> Depending upon the compiler to optimize away the non-existent function
> reference is not ISO-C :-)  Although the fact the Intel compiler isn't
> doing this is amusing.

True :-)

Well it should. There are tons of similar patterns all over the kernel
which use the same trick.
If it didn't optimize  all of these, there would be many more problems.

-Andi

Re: [PATCH] fixes for building kernel using Intel compiler

[Andi Kleen]

On Fri, Oct 18, 2002 at 05:45:08PM -0700, Nakajima, Jun wrote:
> No, it removes most of such cases. It happens only for a general boolean
> controlling expression, and this is the only spot as far as we tested. But

	So it would be optimized away if changed to 

	if ((offsetof(struct task_struct, thread.i387.fxsave) & 15) != 0) {

	?

> our argument is that the checking code is not required because
> thread.i387.fxsave is __attribute__ ((aligned (16))). If __attribute__
> ((aligned (...))) is broken, we should see more problems.

I think it would be better to keep the check, even with attribute aligned. These bugs
are nasty to debug when they happen.

-Andi

RE: [PATCH] fixes for building kernel using Intel compiler

[Nakajima, Jun]

The one we used was 7.0, and it will be available late November or early
December. It's not 6.0 version, which requires more changes.

Looks like I need to resend another patch.

Thanks,
Jun

-----Original Message-----
From: Greg KH [mailto:greg@kroah.com]
Sent: Friday, October 18, 2002 5:08 PM
To: Nakajima, Jun
Cc: Linus Torvalds; Linux Kernel Mailing List; Mallick, Asit K; Saxena,
Sunil
Subject: Re: [PATCH] fixes for building kernel using Intel compiler


On Fri, Oct 18, 2002 at 04:48:34PM -0700, Nakajima, Jun wrote:
> Hi Linus,
> 
> Attached is the patch that resolves some of the redundant code and casting
> that are required to build the Linux kernel usning Intel compiler. We
would
> like to get this patch incorporated to allow the kernel built with Intel
> Compiler.

The patch is wrapped, and can't apply.

And what version of the Intel compiler is this for?  I tried to get a
previous version to build a kernel, but I had to change a lot more
things than you did (like build flags, etc.)

thanks,

greg k-h

RE: [PATCH] fixes for building kernel using Intel compiler

[Nakajima, Jun]

Thanks, Andi.

No, because of (size_t) (it's typedef) from offsetof() and &.  But
	if ( (int) offsetof(struct task_struct, thread.i387.fxsave) % 16) {
would be optiminzed away. So I'll change the patch like that. 

Thanks,
Jun
-----Original Message-----
From: Andi Kleen [mailto:ak@suse.de]
Sent: Friday, October 18, 2002 6:02 PM
To: Nakajima, Jun
Cc: Andi Kleen; David S. Miller; torvalds@transmeta.com;
linux-kernel@vger.kernel.org; Mallick, Asit K; Saxena, Sunil
Subject: Re: [PATCH] fixes for building kernel using Intel compiler


On Fri, Oct 18, 2002 at 05:45:08PM -0700, Nakajima, Jun wrote:
> No, it removes most of such cases. It happens only for a general boolean
> controlling expression, and this is the only spot as far as we tested. But

	So it would be optimized away if changed to 

	if ((offsetof(struct task_struct, thread.i387.fxsave) & 15) != 0) {

	?

> our argument is that the checking code is not required because
> thread.i387.fxsave is __attribute__ ((aligned (16))). If __attribute__
> ((aligned (...))) is broken, we should see more problems.

I think it would be better to keep the check, even with attribute aligned.
These bugs
are nasty to debug when they happen.

-Andi

Re: [PATCH] fixes for building kernel using Intel compiler

[Andi Kleen]

BTW do you have any benchmark / code size results to share with 
Intel compiler vs gcc 3.2 ? How much does it give ?

-Andi

Re: [PATCH] fixes for building kernel using Intel compiler

[Pavel Machek]

Hi!

> BTW do you have any benchmark / code size results to share with 
> Intel compiler vs gcc 3.2 ? How much does it give ?

Also does it work properly after compiling with icc? Working well with
second compiler would be pretty good news for both kernel and icc...

								Pavel
-- 
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?

RE: [PATCH] fixes for building kernel using Intel compiler

[Ulrich Weigand]

Jun Nakajima wrote:

>Having said that, one occasion where people might be surprised by gcc (this
>might be a known issue, though) is: typedef + __attribute__; it ignores
>__attribute__. For example,
>#include <stdio.h>
>
>struct foo_16 {
>        char xxx[3];
>        short yyy;
>} __attribute__ ((aligned (16)));
>
>typedef struct bar_16 {
>        char xxx[3];
>        short yyy;
>} bar_16_t __attribute__ ((aligned (16)));

This is a user error (sort of); you're supposed to write:

typedef struct bar_16 {
        char xxx[3];
        short yyy;
} __attribute__ ((aligned (16))) bar_16_t;

[The attribute modifies the original struct type, which gets then
assigned the typedef name.  This is similar to the case where a
variable definition follows:

struct { ... } __attribute__() var;
vs.
struct { ... ] var __attribute__();

In the first case, the attribute modifies the struct type itself,
while in the second case the attribute applies only to the one
instance var.]

A warning would still be nice; we got bitten by that one a couple
of times ...

>In the kernel, there are several device drivers (ftape-bsm.h, e100.h, for
>example) are doing this kind of thing (i.e. typedef + attribute).

Well, I guess in those files the attribute((packed)) is a no-op
anyway as the structs are already packed according to the default
rules, so it doesn't really matter.  It should probably still be
fixed ...


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: [PATCH] fixes for building kernel using Intel compiler

[Suresh Siddha]

> -----Original Message-----
> From: Ulrich Weigand [mailto:Ulrich.Weigand@de.ibm.com]
> Sent: Monday, October 21, 2002 11:29 AM
> To: jun.nakajima@intel.com
> Cc: linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] fixes for building kernel using Intel compiler
> 
> 
> 
> This is a user error (sort of); you're supposed to write:
> 
> typedef struct bar_16 {
>         char xxx[3];
>         short yyy;
> } __attribute__ ((aligned (16))) bar_16_t;
> 

gcc documentation says
(http://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html#Type%20Attributes) 

<snip> 
You may specify the aligned and transparent_union attributes either in a 
typedef declaration or just past the closing curly brace of a complete 
enum, struct or union type definition and the packed attribute only past 
the closing brace of a definition 
</snip>

We tried with gcc 3.2 and  Juns test case for attribute(aligned) is 
passing. So its just probably some old gcc bug.

> Well, I guess in those files the attribute((packed)) is a no-op
> anyway as the structs are already packed according to the default
> rules, so it doesn't really matter.  It should probably still be
> fixed ...
> 

I have appended a patch for 2.5.44 which fixes this packed attribute 
issue.

thanks,
suresh

diff -Nru linux-2.5.44/arch/s390x/kernel/ptrace32.h attrib-linux/arch/s390x/kernel/ptrace32.h
--- linux-2.5.44/arch/s390x/kernel/ptrace32.h	Fri Oct 18 21:01:52 2002
+++ attrib-linux/arch/s390x/kernel/ptrace32.h	Tue Oct 22 12:03:25 2002
@@ -6,14 +6,14 @@
 typedef struct
 {
 	__u32 cr[3];
-} per_cr_words32  __attribute__((packed));
+} __attribute__((packed)) per_cr_words32;  
 
 typedef struct
 {
 	__u16          perc_atmid;          /* 0x096 */
 	__u32          address;             /* 0x098 */
 	__u8           access_id;           /* 0x0a1 */
-} per_lowcore_words32  __attribute__((packed));
+} __attribute__((packed)) per_lowcore_words32;
 
 typedef struct
 {
@@ -37,7 +37,7 @@
 	union {
 		per_lowcore_words32 words;
 	} lowcore; 
-} per_struct32 __attribute__((packed));
+} __attribute__((packed)) per_struct32;
 
 struct user_regs_struct32
 {
diff -Nru linux-2.5.44/drivers/char/ftape/lowlevel/ftape-bsm.h attrib-linux/drivers/char/ftape/lowlevel/ftape-bsm.h
--- linux-2.5.44/drivers/char/ftape/lowlevel/ftape-bsm.h	Fri Oct 18 21:00:42 2002
+++ attrib-linux/drivers/char/ftape/lowlevel/ftape-bsm.h	Mon Oct 21 15:48:58 2002
@@ -47,7 +47,7 @@
  */
 typedef struct NewSectorMap {          
 	__u8 bytes[3];
-} SectorCount __attribute__((packed));
+} __attribute__((packed)) SectorCount;
 
 
 /*
diff -Nru linux-2.5.44/drivers/net/e100/e100.h attrib-linux/drivers/net/e100/e100.h
--- linux-2.5.44/drivers/net/e100/e100.h	Fri Oct 18 21:01:20 2002
+++ attrib-linux/drivers/net/e100/e100.h	Mon Oct 21 15:45:04 2002
@@ -488,7 +488,7 @@
 	u8 scb_fc_thld;	/* Flow Control threshold */
 	u8 scb_fc_xon_xoff;	/* Flow Control XON/XOFF values */
 	u8 scb_pmdr;	/* Power Mgmt. Driver Reg */
-} d101_scb_ext __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) d101_scb_ext;
 
 /* Changed for 82559 enhancement */
 typedef struct _d101m_scb_ext_t {
@@ -504,7 +504,7 @@
 	u32 scb_function_event_mask;	/* Cardbus Function Mask */
 	u32 scb_function_present_state;	/* Cardbus Function state */
 	u32 scb_force_event;	/* Cardbus Force Event */
-} d101m_scb_ext __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) d101m_scb_ext; 
 
 /* Changed for 82550 enhancement */
 typedef struct _d102_scb_ext_t {
@@ -523,7 +523,7 @@
 	u32 scb_function_event_mask;	/* Cardbus Function Mask */
 	u32 scb_function_present_state;	/* Cardbus Function state */
 	u32 scb_force_event;	/* Cardbus Force Event */
-} d102_scb_ext __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) d102_scb_ext;
 
 /*
  * 82557 status control block. this will be memory mapped & will hang of the
@@ -545,7 +545,7 @@
 		d101m_scb_ext d101m_scb;	/* 82559 specific fields */
 		d102_scb_ext d102_scb;
 	} scb_ext;
-} scb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) scb_t;
 
 /* Self test
  * This is used to dump results of the self test 
@@ -553,7 +553,7 @@
 typedef struct _self_test_t {
 	u32 st_sign;	/* Self Test Signature */
 	u32 st_result;	/* Self Test Results */
-} self_test_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) self_test_t;
 
 /* 
  *  Statistical Counters 
@@ -636,39 +636,39 @@
 	u16 cb_status;	/* Command Block Status */
 	u16 cb_cmd;	/* Command Block Command */
 	u32 cb_lnk_ptr;	/* Link To Next CB */
-} cb_header_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) cb_header_t;
 
 //* Individual Address Command Block (IA_CB)*/
 typedef struct _ia_cb_t {
 	cb_header_t ia_cb_hdr;
 	u8 ia_addr[ETH_ALEN];
-} ia_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) ia_cb_t;
 
 /* Configure Command Block (CONFIG_CB)*/
 typedef struct _config_cb_t {
 	cb_header_t cfg_cbhdr;
 	u8 cfg_byte[CB_CFIG_BYTE_COUNT + CB_CFIG_D102_BYTE_COUNT];
-} config_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) config_cb_t;
 
 /* MultiCast Command Block (MULTICAST_CB)*/
 typedef struct _multicast_cb_t {
 	cb_header_t mc_cbhdr;
 	u16 mc_count;	/* Number of multicast addresses */
 	u8 mc_addr[(ETH_ALEN * MAX_MULTICAST_ADDRS)];
-} mltcst_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) mltcst_cb_t; 
 
 #define UCODE_MAX_DWORDS	134
 /* Load Microcode Command Block (LOAD_UCODE_CB)*/
 typedef struct _load_ucode_cb_t {
 	cb_header_t load_ucode_cbhdr;
 	u32 ucode_dword[UCODE_MAX_DWORDS];
-} load_ucode_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) load_ucode_cb_t;
 
 /* Load Programmable Filter Data*/
 typedef struct _filter_cb_t {
 	cb_header_t filter_cb_hdr;
 	u32 filter_data[MAX_FILTER];
-} filter_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) filter_cb_t;
 
 /* NON_TRANSMIT_CB -- Generic Non-Transmit Command Block 
  */
@@ -680,7 +680,7 @@
 		mltcst_cb_t multicast;
 		filter_cb_t filter;
 	} ntcb;
-} nxmit_cb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) nxmit_cb_t;
 
 /*Block for queuing for postponed execution of the non-transmit commands*/
 typedef struct _nxmit_cb_entry_t {
@@ -713,7 +713,7 @@
 	u32 tbd_buf_addr;	/* Physical Transmit Buffer Address */
 	u16 tbd_buf_cnt;	/* Actual Count Of Bytes */
 	u16 padd;
-} tbd_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) tbd_t;
 
 /* d102 specific fields */
 typedef struct _tcb_ipcb_t {
@@ -732,7 +732,7 @@
 		u16 tbd_zero_size;
 	} tbd_sec_size;
 	u16 total_tcp_payload;
-} tcb_ipcb_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) tcb_ipcb_t;
 
 #define E100_TBD_ARRAY_SIZE (2+MAX_SKB_FRAGS)
 
@@ -795,7 +795,7 @@
 	u32 rbd_rcb_addr;	/* Receive Buffer Address */
 	u16 rbd_sz;	/* Receive Buffer Size */
 	u16 rbd_filler1;
-} rbd_t __attribute__ ((__packed__));
+} __attribute__ ((__packed__)) rbd_t;
 
 /*
  * This structure is used to maintain a FIFO access to a resource that is 
diff -Nru linux-2.5.44/drivers/net/gt96100eth.h attrib-linux/drivers/net/gt96100eth.h
--- linux-2.5.44/drivers/net/gt96100eth.h	Fri Oct 18 21:01:17 2002
+++ attrib-linux/drivers/net/gt96100eth.h	Mon Oct 21 15:46:17 2002
@@ -214,7 +214,7 @@
 	u32 cmdstat;
 	u32 next;
 	u32 buff_ptr;
-} gt96100_td_t __attribute__ ((packed));
+}__attribute__ ((packed)) gt96100_td_t;
 
 typedef struct {
 #ifdef DESC_BE
@@ -227,7 +227,7 @@
 	u32 cmdstat;
 	u32 next;
 	u32 buff_ptr;
-} gt96100_rd_t __attribute__ ((packed));
+} __attribute__ ((packed)) gt96100_rd_t;
 
 
 /* Values for the Tx command-status descriptor entry. */
diff -Nru linux-2.5.44/drivers/s390/net/lcs.c attrib-linux/drivers/s390/net/lcs.c
--- linux-2.5.44/drivers/s390/net/lcs.c	Fri Oct 18 21:01:13 2002
+++ attrib-linux/drivers/s390/net/lcs.c	Mon Oct 21 15:54:58 2002
@@ -322,7 +322,7 @@
 u8		slot;
 
 typedef struct {
-lcs_header} lcs_header_type __attribute__ ((packed));
+lcs_header} __attribute__ ((packed)) lcs_header_type;
 
 enum {
 	lcs_390_initiated,
@@ -345,13 +345,13 @@
 	u8 operator_flags[3];
 	u8 reserved[3];
 	u8 command_data[0];
-} lcs_std_cmd __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_std_cmd; 
 
 typedef struct {
 	lcs_header lcs_base_cmd u16 unused1;
 	u16 buff_size;
 	u8 unused2[6];
-} lcs_startup_cmd __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_startup_cmd;
 
 #define LCS_ADDR_LEN 6
 
@@ -360,7 +360,7 @@
 	u32 ip_addr;
 	u8 mac_address[LCS_ADDR_LEN];
 	u8 reserved[2];
-} lcs_ip_mac_addr_pair __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_ip_mac_addr_pair; 
 
 typedef enum {
 	ipm_set_required,
@@ -385,7 +385,7 @@
 	u16 version;		/* IP version i.e. 4 */
 	lcs_ip_mac_addr_pair ip_mac_pair[MAX_IP_MAC_PAIRS];
 	u32 response_data;
-} lcs_ipm_ctlmsg __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_ipm_ctlmsg;
 typedef struct {
 	lcs_header lcs_base_cmd u8 lan_type;
 	u8 rel_adapter_no;
@@ -393,7 +393,7 @@
 	u16 ip_assists_supported;	/* returned by OSA  */
 	u16 ip_assists_enabled;	/* returned by OSA */
 	u16 version;		/* IP version i.e. 4 */
-} lcs_qipassist_ctlmsg __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_qipassist_ctlmsg;
 
 typedef enum {
 	/* Not supported by LCS */
@@ -425,7 +425,7 @@
 	u32 num_rx_errors_detected;
 	u32 num_rx_discarded_nobuffs_avail;
 	u32 num_rx_packets_too_large;
-} lcs_lanstat_reply __attribute__ ((packed));
+} __attribute__ ((packed)) lcs_lanstat_reply;
 
 /* This buffer sizes are used by MVS so they should be reasonable */
 #define LCS_IOBUFFSIZE     0x5000
diff -Nru linux-2.5.44/include/linux/acpi.h attrib-linux/include/linux/acpi.h
--- linux-2.5.44/include/linux/acpi.h	Fri Oct 18 21:00:42 2002
+++ attrib-linux/include/linux/acpi.h	Mon Oct 21 17:31:31 2002
@@ -90,7 +90,7 @@
 typedef struct {
 	u8			type;
 	u8			length;
-} acpi_table_entry_header __attribute__ ((packed));
+} __attribute__ ((packed)) acpi_table_entry_header;
 
 /* Root System Description Table (RSDT) */
 
@@ -143,7 +143,7 @@
 	u16			polarity:2;
 	u16			trigger:2;
 	u16			reserved:12;
-} acpi_interrupt_flags __attribute__ ((packed));
+} __attribute__ ((packed)) acpi_interrupt_flags;
 
 struct acpi_table_lapic {
 	acpi_table_entry_header	header;
diff -Nru linux-2.5.44/include/linux/sctp.h attrib-linux/include/linux/sctp.h
--- linux-2.5.44/include/linux/sctp.h	Fri Oct 18 21:01:59 2002
+++ attrib-linux/include/linux/sctp.h	Mon Oct 21 17:36:35 2002
@@ -59,14 +59,14 @@
 	__u16 dest;
 	__u32 vtag;
 	__u32 checksum;
-} sctp_sctphdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_sctphdr_t;
 
 /* Section 3.2.  Chunk Field Descriptions. */
 typedef struct sctp_chunkhdr {
 	__u8 type;
 	__u8 flags;
 	__u16 length;
-} sctp_chunkhdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_chunkhdr_t;
 
 
 /* Section 3.2.  Chunk Type Values.
@@ -150,7 +150,7 @@
 typedef struct sctp_paramhdr {
 	__u16 type;
 	__u16 length;
-} sctp_paramhdr_t __attribute((packed));
+} __attribute((packed)) sctp_paramhdr_t; 
 
 typedef enum {
 
@@ -200,12 +200,12 @@
 	__u16 ssn;
 	__u32 ppid;
 	__u8  payload[0];
-} sctp_datahdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_datahdr_t;
 
 typedef struct sctp_data_chunk {
         sctp_chunkhdr_t chunk_hdr;
         sctp_datahdr_t  data_hdr;
-} sctp_data_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_data_chunk_t;
 
 /* DATA Chuck Specific Flags */
 enum {
@@ -230,48 +230,48 @@
 	__u16 num_inbound_streams;
 	__u32 initial_tsn;
 	__u8  params[0];
-} sctp_inithdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_inithdr_t;
 
 typedef struct sctp_init_chunk {
 	sctp_chunkhdr_t chunk_hdr;
 	sctp_inithdr_t init_hdr;
-} sctp_init_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_init_chunk_t;
 
 
 /* Section 3.3.2.1. IPv4 Address Parameter (5) */
 typedef struct sctp_ipv4addr_param {
 	sctp_paramhdr_t param_hdr;
 	struct in_addr  addr;
-} sctp_ipv4addr_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_ipv4addr_param_t;
 
 /* Section 3.3.2.1. IPv6 Address Parameter (6) */
 typedef struct sctp_ipv6addr_param {
 	sctp_paramhdr_t param_hdr;
 	struct in6_addr addr;
-} sctp_ipv6addr_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_ipv6addr_param_t;
 	
 /* Section 3.3.2.1 Cookie Preservative (9) */
 typedef struct sctp_cookie_preserve_param {
 	sctp_paramhdr_t param_hdr;
 	uint32_t        lifespan_increment;
-} sctp_cookie_preserve_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_cookie_preserve_param_t;
 
 /* Section 3.3.2.1 Host Name Address (11) */
 typedef struct sctp_hostname_param {
 	sctp_paramhdr_t param_hdr;
 	uint8_t hostname[0];
-} sctp_hostname_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_hostname_param_t;
 
 /* Section 3.3.2.1 Supported Address Types (12) */
 typedef struct sctp_supported_addrs_param {
 	sctp_paramhdr_t param_hdr;
 	uint16_t types[0];
-} sctp_supported_addrs_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_supported_addrs_param_t;
 
 /* Appendix A. ECN Capable (32768) */
 typedef struct sctp_ecn_capable_param {
 	sctp_paramhdr_t param_hdr;
-} sctp_ecn_capable_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_ecn_capable_param_t;
 
 
 
@@ -285,13 +285,13 @@
 typedef struct sctp_cookie_param {
 	sctp_paramhdr_t p;
 	__u8 body[0];
-} sctp_cookie_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_cookie_param_t;
 
 /* Section 3.3.3.1 Unrecognized Parameters (8) */
 typedef struct sctp_unrecognized_param {
 	sctp_paramhdr_t param_hdr;
 	sctp_paramhdr_t unrecognized;
-} sctp_unrecognized_param_t __attribute__((packed));
+} __attribute__((packed)) sctp_unrecognized_param_t;
 
 
 
@@ -306,7 +306,7 @@
 typedef struct sctp_gap_ack_block {
 	__u16 start;
 	__u16 end;
-} sctp_gap_ack_block_t __attribute__((packed));
+} __attribute__((packed)) sctp_gap_ack_block_t;
 
 typedef uint32_t sctp_dup_tsn_t;
 
@@ -321,12 +321,12 @@
 	__u16 num_gap_ack_blocks;
 	__u16 num_dup_tsns;
 	sctp_sack_variable_t variable[0];
-} sctp_sackhdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_sackhdr_t;
 
 typedef struct sctp_sack_chunk {
 	sctp_chunkhdr_t chunk_hdr;
 	sctp_sackhdr_t sack_hdr;
-} sctp_sack_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_sack_chunk_t;
 
 
 /* RFC 2960.  Section 3.3.5 Heartbeat Request (HEARTBEAT) (4):
@@ -338,12 +338,12 @@
 
 typedef struct sctp_heartbeathdr {
 	sctp_paramhdr_t info;
-} sctp_heartbeathdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_heartbeathdr_t; 
 
 typedef struct sctp_heartbeat_chunk {
 	sctp_chunkhdr_t chunk_hdr;
 	sctp_heartbeathdr_t hb_hdr;
-} sctp_heartbeat_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_heartbeat_chunk_t;
 	
   
 /* For the abort and shutdown ACK we must carry the init tag in the
@@ -352,7 +352,7 @@
  */
 typedef struct sctp_abort_chunk {
         sctp_chunkhdr_t uh;
-} sctp_abort_chunkt_t __attribute__((packed));
+} __attribute__((packed)) sctp_abort_chunkt_t; 
 
 
 /* For the graceful shutdown we must carry the tag (in common header)
@@ -360,7 +360,7 @@
  */
 typedef struct sctp_shutdownhdr {
 	__u32 cum_tsn_ack;
-} sctp_shutdownhdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_shutdownhdr_t;
 
 struct sctp_shutdown_chunk_t {
         sctp_chunkhdr_t    chunk_hdr;
@@ -375,12 +375,12 @@
 	__u16 cause;
 	__u16 length;
 	__u8  variable[0];
-} sctp_errhdr_t __attribute__((packed));
+} __attribute__((packed)) sctp_errhdr_t;
 
 typedef struct sctp_operr_chunk {
         sctp_chunkhdr_t chunk_hdr;
 	sctp_errhdr_t   err_hdr;
-} sctp_operr_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_operr_chunk_t;
 
 /* RFC 2960 3.3.10 - Operation Error
  *
@@ -455,7 +455,7 @@
 typedef struct sctp_ecne_chunk {
 	sctp_chunkhdr_t chunk_hdr;
 	sctp_ecnehdr_t ence_hdr;
-} sctp_ecne_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_ecne_chunk_t;
 
 /* RFC 2960.  Appendix A.  Explicit Congestion Notification. 
  *   Congestion Window Reduced (CWR) (13)
@@ -467,7 +467,7 @@
 typedef struct sctp_cwr_chunk {
 	sctp_chunkhdr_t chunk_hdr;
 	sctp_cwrhdr_t cwr_hdr;
-} sctp_cwr_chunk_t __attribute__((packed));
+} __attribute__((packed)) sctp_cwr_chunk_t;
 
 
 /* FIXME:  Cleanup needs to continue below this line. */
diff -Nru linux-2.5.44/include/net/sctp/structs.h attrib-linux/include/net/sctp/structs.h
--- linux-2.5.44/include/net/sctp/structs.h	Fri Oct 18 21:02:34 2002
+++ attrib-linux/include/net/sctp/structs.h	Mon Oct 21 17:37:46 2002
@@ -391,7 +391,7 @@
 	sctp_paramhdr_t param_hdr;
 	sockaddr_storage_t daddr;
 	unsigned long sent_at;
-} sctp_sender_hb_info_t __attribute__((packed));
+} __attribute__((packed)) sctp_sender_hb_info_t;
 
 /* RFC2960 1.4 Key Terms
  *