Re: const versus __attribute__((const))

Re: const versus __attribute__((const))

[Arnd Bergmann]

H. Peter Anvin writes:
> I have made a patch against the current tree defining
> __attribute_const__ in <linux/compiler.h> and using it in the above
> cases; does anyone know any reason why I should *NOT* submit this to
> Linus?

I noticed before that gcc appearantly ignores __attribute__((const))
for inline functions, so both the original and your proposed code
is rather pointless as an optimization, except for extern declarations.

I'd rather remove the 'const' completely where it causes warnings for
inline functions.

	Arnd <><

Re: const versus __attribute__((const))

[H. Peter Anvin]

Arnd Bergmann wrote:
> H. Peter Anvin writes:
> 
>>I have made a patch against the current tree defining
>>__attribute_const__ in <linux/compiler.h> and using it in the above
>>cases; does anyone know any reason why I should *NOT* submit this to
>>Linus?
> 
> 
> I noticed before that gcc appearantly ignores __attribute__((const))
> for inline functions, so both the original and your proposed code
> is rather pointless as an optimization, except for extern declarations.
> 
> I'd rather remove the 'const' completely where it causes warnings for
> inline functions.
> 

These functions are available to userspace, though, and can be compiled 
with -O0; thus not inlined.

	-hpa

Re: const versus __attribute__((const))

[Nikita Danilov]

H. Peter Anvin writes:
 > Arnd Bergmann wrote:
 > > H. Peter Anvin writes:
 > > 
 > >>I have made a patch against the current tree defining
 > >>__attribute_const__ in <linux/compiler.h> and using it in the above
 > >>cases; does anyone know any reason why I should *NOT* submit this to
 > >>Linus?
 > > 
 > > 
 > > I noticed before that gcc appearantly ignores __attribute__((const))
 > > for inline functions, so both the original and your proposed code
 > > is rather pointless as an optimization, except for extern declarations.
 > > 
 > > I'd rather remove the 'const' completely where it causes warnings for
 > > inline functions.
 > > 
 > 
 > These functions are available to userspace, though, and can be compiled 
 > with -O0; thus not inlined.

And future versions of gcc can be smarter.

 > 
 > 	-hpa

Nikita.

Re: const versus __attribute__((const))

[H. Peter Anvin]

Nikita Danilov wrote:
>  > 
>  > These functions are available to userspace, though, and can be compiled 
>  > with -O0; thus not inlined.
> 
> And future versions of gcc can be smarter.
> 

Actually, the reason it doesn't use it for the inlines is because it 
doesn't need to -- it already has full visibility, so it doesn't need it 
to be spelled out.

So it would be an issue if gcc got dumber, not smarter.

	-hpa

Re: const versus __attribute__((const))

[Jamie Lokier]

H. Peter Anvin wrote:
> Actually, the reason it doesn't use it for the inlines is because it 
> doesn't need to -- it already has full visibility, so it doesn't need it 
> to be spelled out.

Until a few minutes ago, I disagreed.  I thought GCC wouldn't
eliminate calls to inline functions which contain an inline asm, such
as current_thread_info().

But having just looked, multiple calls to current_thread_info() are
eliminated.  GCC inspects the arguments of the inline asm which is the
body of this function, and concludes that the asm itself is to be
optimised like a const function, depending only on its input operands.

In fact to get GCC to _not_ optimise away inline asms, or even to
behave like "pure" (allowing memory to be read) instead of "const",
they must be declared volatile, or have no outputs.  Putting "memory"
in the clobber list says only that the asm clobbers memory, not that
it reads memory.

It would be nice to have a way to declare an asm like "pure" not
"const", so that it's allowed to read memory but multiple calls can be
eliminated; I don't know of a way to express that.

-- Jamie

Re: const versus __attribute__((const))

[H. Peter Anvin]

Jamie Lokier wrote:
> 
> It would be nice to have a way to declare an asm like "pure" not
> "const", so that it's allowed to read memory but multiple calls can be
> eliminated; I don't know of a way to express that.
> 

Just specify memory input operands.

	-hpa

Re: const versus __attribute__((const))

[Jamie Lokier]

H. Peter Anvin wrote:
> > It would be nice to have a way to declare an asm like "pure" not
> > "const", so that it's allowed to read memory but multiple calls can be
> > eliminated; I don't know of a way to express that.
> 
> Just specify memory input operands.

Thanks.  That's even more useful than "pure" because it implies the
asm only reads the explicitly passed memory operands.

Memory input operands don't work if you want the asm to read arbitrary
memory not mentioned in the inputs (like "pure" allows) or traverse
linked lists.

(A long time ago there was a question about whether GCC could ever
copy the value associated with an "m" operand to a stack slot, and
pass the address of the stack slot.  After all, GCC _will_ copy the
value if the operand is an "r", and presumably gives mixed results
with "rm".  We seem to have concluded that it never will).

-- Jamie

Re: const versus __attribute__((const))

[H. Peter Anvin]

Jamie Lokier wrote:
> H. Peter Anvin wrote:
> 
>>>It would be nice to have a way to declare an asm like "pure" not
>>>"const", so that it's allowed to read memory but multiple calls can be
>>>eliminated; I don't know of a way to express that.
>>
>>Just specify memory input operands.
> 
> 
> Thanks.  That's even more useful than "pure" because it implies the
> asm only reads the explicitly passed memory operands.
> 
> Memory input operands don't work if you want the asm to read arbitrary
> memory not mentioned in the inputs (like "pure" allows) or traverse
> linked lists.
> 
> (A long time ago there was a question about whether GCC could ever
> copy the value associated with an "m" operand to a stack slot, and
> pass the address of the stack slot.  After all, GCC _will_ copy the
> value if the operand is an "r", and presumably gives mixed results
> with "rm".  We seem to have concluded that it never will).
> 

Sure it will:


int foo(int x)
{
   int y, z;

   asm("movl %1,%0" : "=r" (y) : "r" (x));
   asm("movl %1,%0" : "=r" (z) : "m" (y));

   return z;
}

: smyrno 21 ; gcc -O2 -c testme.c
: smyrno 22 ; objdump -dr testme.o

testme.o:     file format elf32-i386

Disassembly of section .text:

00000000 <foo>:
    0:   55                      push   %ebp			; Make stack frame
    1:   89 e5                   mov    %esp,%ebp		; d:o
    3:   50                      push   %eax			; Allocate stack slot
    4:   8b 45 08                mov    0x8(%ebp),%eax		; Copy to register
    7:   89 c0                   mov    %eax,%eax		; First asm()
    9:   89 45 fc                mov    %eax,0xfffffffc(%ebp) 	; Copy to stack slot
    c:   8b 45 fc                mov    0xfffffffc(%ebp),%eax	; Second asm()
    f:   c9                      leave				; Destroy stack frame
   10:   c3                      ret

Re: const versus __attribute__((const))

[Linus Torvalds]

On Tue, 9 Dec 2003, Jamie Lokier wrote:
>
> (A long time ago there was a question about whether GCC could ever
> copy the value associated with an "m" operand to a stack slot, and
> pass the address of the stack slot.  After all, GCC _will_ copy the
> value if the operand is an "r", and presumably gives mixed results
> with "rm".  We seem to have concluded that it never will).

We never never concluded that they never would, but we did (I think)
convince the gcc people that a memory operand to an asm should always be
considered a lvalue. That will effectively mean that we know a memory op
will never be moved around - because then it wouldn't be the same lvalue
any more (a lvalue is literally defined by its address).

So yes, I think we can depend on it now, although we historically
couldn't.

		Linus

Re: const versus __attribute__((const))

[Linus Torvalds]

On Mon, 8 Dec 2003, H. Peter Anvin wrote:
>
> Sure it will:

No, Jamie was thinking of a variable that _already_ has a memory location
having its value copied around to _another_ memory location (ie a
temporary stack slot).

What your example shows is that gcc will create a stack slot for an
automatic variable if it doesn't have one already, in order to make it an
lvalue with a memory address.

So what's interesting is if gcc would take something like

	extern int variable;

	asm("..." : :"m" (variable));

and cause this to create a _new_ temporary on the stack slot, and pass the
asm the pointer to that temporary rather than the "real" address of
'variable'.

And as far as I know, it won't. Newer gcc's will _require_ memory
arguments to be lvalues (well, it will warn if they aren't), and will
always turn them into addressables of that particular lvalue.

This is actually an issue, because some asm code depends on getting the
proper address - not just the proper value. Things like locks etc care
_deeply_ about more than just the value.

		Linus

Re: const versus __attribute__((const))

[H. Peter Anvin]

Linus Torvalds wrote:
> 
> And as far as I know, it won't. Newer gcc's will _require_ memory
> arguments to be lvalues (well, it will warn if they aren't), and will
> always turn them into addressables of that particular lvalue.
> 

Well, I get no warning from the following code, with gcc 3.2.2 and -W -Wall:

int foo(int x)
{
   int y, z, w;

   asm("movl %1,%0" : "=r" (y) : "r" (x));
   asm("movl %1,%0" : "=r" (z) : "m" (y+1));
   asm("movl %1,%0" : "=r" (w) : "m" (33));

   return z+w;
}

The memory operand is definitely not an lvalue.  gcc allocates a stack slot
for the y+1 value as a temporary automatic and passes it in.  The value 33
is put in the .rodata segment:

00000000 <foo>:
    0:   55                      push   %ebp
    1:   89 e5                   mov    %esp,%ebp
    3:   50                      push   %eax
    4:   8b 45 08                mov    0x8(%ebp),%eax
    7:   89 c0                   mov    %eax,%eax
    9:   40                      inc    %eax
    a:   89 45 fc                mov    %eax,0xfffffffc(%ebp)
    d:   8b 15 00 00 00 00       mov    0x0,%edx
                         f: R_386_32     .rodata.cst4
   13:   8b 45 fc                mov    0xfffffffc(%ebp),%eax
   16:   01 d0                   add    %edx,%eax
   18:   c9                      leave
   19:   c3                      ret

> This is actually an issue, because some asm code depends on getting the
> proper address - not just the proper value. Things like locks etc care
> _deeply_ about more than just the value.

That would be a very bad thing indeed.  Fortunately I think it would take
some rather odd contortions on the part of the compiler for that situation
to occur at all, I would think, and if the gcc people have since been
made aware of it it should be pretty easy for them to make sure it will not
happen.

	-hpa

Re: const versus __attribute__((const))

[Arnd Bergmann]

On Tuesday 09 December 2003 08:40, H. Peter Anvin wrote:
> Well, I get no warning from the following code, with gcc 3.2.2 and -W
> -Wall:
>
> int foo(int x)
> {
>    int y, z, w;
>
>    asm("movl %1,%0" : "=r" (y) : "r" (x));
>    asm("movl %1,%0" : "=r" (z) : "m" (y+1));
>    asm("movl %1,%0" : "=r" (w) : "m" (33));
>
>    return z+w;
> }

For reference, both gcc-3.3 and gcc-3.4 (snapshot) give produce the same assembly 
as gcc-3.2 for your code, but give this warning:

test.c:6: warning: use of memory input without lvalue in asm operand 1 is deprecated
test.c:7: warning: use of memory input without lvalue in asm operand 1 is deprecated

	Arnd <><

Re: const versus __attribute__((const))

[H. Peter Anvin]

Arnd Bergmann wrote:
> 
> For reference, both gcc-3.3 and gcc-3.4 (snapshot) give produce the same assembly 
> as gcc-3.2 for your code, but give this warning:
> 
> test.c:6: warning: use of memory input without lvalue in asm operand 1 is deprecated
> test.c:7: warning: use of memory input without lvalue in asm operand 1 is deprecated
> 

In some ways, this is rather unfortunate, too.  What it really means is 
that the gcc "m" constraint is overloaded; it would have been better if 
they would have created a new modifier (say "*") for "must be lvalue."

	-hpa

Re: const versus __attribute__((const))

[Linus Torvalds]

On Tue, 9 Dec 2003, H. Peter Anvin wrote:
>
> In some ways, this is rather unfortunate, too.  What it really means is
> that the gcc "m" constraint is overloaded; it would have been better if
> they would have created a new modifier (say "*") for "must be lvalue."

The thing is, most users of "m" (like 99%) actually mean "_THIS_ memory
location". So just fixing the "m" modifier was an easy way to make sure
that users get the behaviour they expect.

Also, I have this dim memory of there actually being a potential bug in
"m" handling inside gcc, and requiring the entry to be a lvalue was the
easiest way to fix it. Richard Henderson would have the details.  I think
it was the liveness analysis that got confused or something.

And the thing is, if you have a non-lvalue right now, you will (a) get a
nice warnign that tells you so, and (b) it will be trivial to fix. So
something like

	asm("xxxx" : :"m" (1+x));

can be trivially fixed to be

	{
		int tmp = 1+x;
		asm("xxxx" : : "m" (tmp));
	}

so it's not like it's a horribly undue burden on the programmer.

In the kernel, I don't think we had a _single_ case that needed this, but
I might remember that wrong. Anyway, it wasn't a problem - and the kernel
tends to be the single most active user of inline asm's of all
gcc-compiled projects.

			Linus

Re: const versus __attribute__((const))

[H. Peter Anvin]

Linus Torvalds wrote:
> 
> On Tue, 9 Dec 2003, H. Peter Anvin wrote:
> 
>>In some ways, this is rather unfortunate, too.  What it really means is
>>that the gcc "m" constraint is overloaded; it would have been better if
>>they would have created a new modifier (say "*") for "must be lvalue."
> 
> 
> The thing is, most users of "m" (like 99%) actually mean "_THIS_ memory
> location". So just fixing the "m" modifier was an easy way to make sure
> that users get the behaviour they expect.
> 

Agreed.  It's just a bit ugly that the "m" in "rm" has a different 
meaning than just "m".

	-hpa

Re: const versus __attribute__((const))

[Jamie Lokier]

H. Peter Anvin wrote:
> Agreed.  It's just a bit ugly that the "m" in "rm" has a different 
> meaning than just "m".

Oh?  What does the "m" in "m,m" mean then?

(I don't have GCC 3.3 to try it here).

Sometimes that's useful, when used with differing multi-alternative
constraints on the other arguments, and 99% of the time you _do_ want
the "m,m" constraint to use the actual lvalue's address just like "m".

-- Jamie

Re: const versus __attribute__((const))

[Nikita Danilov]

H. Peter Anvin writes:
 > I have been chasing down a bunch of warnings that have been annoying
 > me, and I have observed that a bunch of the byteorder functions are
 > defined in ways similar to:
 > 
 > static __inline__ __const__ __u16 ___arch__swab16(__u16 value)
 > 
 > With -W -Wall at least gcc 3.2.2 will issue a warning:
 > 
 > warning: type qualifiers ignored on function return type
 > 
 > ... which seems to imply the __const__ is ignored.  Reading the gcc
 > documentation it appears the correct syntax is
 > __attribute__((__const__)) rather than __const__.
 > 
 > I have made a patch against the current tree defining
 > __attribute_const__ in <linux/compiler.h> and using it in the above
 > cases; does anyone know any reason why I should *NOT* submit this to
 > Linus?

How about this one: I sent such patch (see below) some time ago and it
did not avail?

 > 
 > 	-hpa

Nikita.

diff -puN include/asm-arm/current.h~const_fn include/asm-arm/current.h
--- i386/include/asm-arm/current.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-arm/current.h	Thu Nov 27 15:22:58 2003
@@ -2,8 +2,9 @@
 #define _ASMARM_CURRENT_H
 
 #include <linux/thread_info.h>
+#include <linux/compiler.h>
 
-static inline struct task_struct *get_current(void) __attribute__ (( __const__ ));
+static inline struct task_struct *get_current(void) __attribute_const__;
 
 static inline struct task_struct *get_current(void)
 {
diff -puN include/asm-arm/thread_info.h~const_fn include/asm-arm/thread_info.h
--- i386/include/asm-arm/thread_info.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-arm/thread_info.h	Thu Nov 27 15:22:58 2003
@@ -76,7 +76,7 @@ struct thread_info {
 /*
  * how to get the thread information struct from C
  */
-static inline struct thread_info *current_thread_info(void) __attribute__ (( __const__ ));
+static inline struct thread_info *current_thread_info(void) __const_fn;
 
 static inline struct thread_info *current_thread_info(void)
 {
diff -puN include/asm-i386/byteorder.h~const_fn include/asm-i386/byteorder.h
--- i386/include/asm-i386/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-i386/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -10,7 +10,9 @@
 #include <linux/config.h>
 #endif
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
+#include <linux/compiler.h>
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x) 
 {
 #ifdef CONFIG_X86_BSWAP
 	__asm__("bswap %0" : "=r" (x) : "0" (x));
@@ -26,7 +28,7 @@ static __inline__ __const__ __u32 ___arc
 
 /* gcc should generate this for open coded C now too. May be worth switching to 
    it because inline assembly cannot be scheduled. -AK */
-static __inline__ __const__ __u16 ___arch__swab16(__u16 x)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
 {
 	__asm__("xchgb %b0,%h0"		/* swap bytes		*/
 		: "=q" (x)
@@ -35,7 +37,7 @@ static __inline__ __const__ __u16 ___arc
 }
 
 
-static inline __u64 ___arch__swab64(__u64 val) 
+static inline __attribute_const__ __u64 ___arch__swab64(__u64 val) 
 { 
 	union { 
 		struct { __u32 a,b; } s;
diff -puN include/asm-ia64/byteorder.h~const_fn include/asm-ia64/byteorder.h
--- i386/include/asm-ia64/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-ia64/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -8,8 +8,9 @@
 
 #include <asm/types.h>
 #include <asm/intrinsics.h>
-
-static __inline__ __const__ __u64
+#include <linux/compiler.h>
+  
+static __inline__ __attribute_const__ __u64
 __ia64_swab64 (__u64 x)
 {
 	__u64 result;
@@ -18,13 +19,13 @@ __ia64_swab64 (__u64 x)
 	return result;
 }
 
-static __inline__ __const__ __u32
+static __inline__ __attribute_const__ __u32
 __ia64_swab32 (__u32 x)
 {
 	return __ia64_swab64(x) >> 32;
 }
 
-static __inline__ __const__ __u16
+static __inline__ __attribute_const__ __u16
 __ia64_swab16(__u16 x)
 {
 	return __ia64_swab64(x) >> 48;
diff -puN include/asm-m68k/byteorder.h~const_fn include/asm-m68k/byteorder.h
--- i386/include/asm-m68k/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-m68k/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -5,7 +5,9 @@
 
 #ifdef __GNUC__
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 val)
+#include <linux/compiler.h>
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 val)
 {
 	__asm__("rolw #8,%0; swap %0; rolw #8,%0" : "=d" (val) : "0" (val));
 	return val;
diff -puN include/asm-parisc/byteorder.h~const_fn include/asm-parisc/byteorder.h
--- i386/include/asm-parisc/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-parisc/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -5,7 +5,7 @@
 
 #ifdef __GNUC__
 
-static __inline__ __const__ __u16 ___arch__swab16(__u16 x)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
 {
 	__asm__("dep %0, 15, 8, %0\n\t"		/* deposit 00ab -> 0bab */
 		"shd %%r0, %0, 8, %0"		/* shift 000000ab -> 00ba */
@@ -14,7 +14,7 @@ static __inline__ __const__ __u16 ___arc
 	return x;
 }
 
-static __inline__ __const__ __u32 ___arch__swab24(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab24(__u32 x)
 {
 	__asm__("shd %0, %0, 8, %0\n\t"		/* shift xabcxabc -> cxab */
 		"dep %0, 15, 8, %0\n\t"		/* deposit cxab -> cbab */
@@ -24,7 +24,9 @@ static __inline__ __const__ __u32 ___arc
 	return x;
 }
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
+#include <linux/compiler.h>
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
 	unsigned int temp;
 	__asm__("shd %0, %0, 16, %1\n\t"	/* shift abcdabcd -> cdab */
@@ -47,7 +49,7 @@ static __inline__ __const__ __u32 ___arc
 **      HSHR    67452301 -> *6*4*2*0 into %0
 **      OR      %0 | %1  -> 76543210 into %0 (all done!)
 */
-static __inline__ __const__ __u64 ___arch__swab64(__u64 x) {
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 x) {
 	__u64 temp;
 	__asm__("permh,3210 %0, %0\n\t"
 		"hshl %0, 8, %1\n\t"
@@ -60,7 +62,7 @@ static __inline__ __const__ __u64 ___arc
 #define __arch__swab64(x) ___arch__swab64(x)
 #define __BYTEORDER_HAS_U64__
 #elif !defined(__STRICT_ANSI__)
-static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 x)
 {
 	__u32 t1 = ___arch__swab32((__u32) x);
 	__u32 t2 = ___arch__swab32((__u32) (x >> 32));
diff -puN include/asm-ppc/byteorder.h~const_fn include/asm-ppc/byteorder.h
--- i386/include/asm-ppc/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-ppc/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -4,8 +4,11 @@
 #include <asm/types.h>
 
 #ifdef __GNUC__
+
 #ifdef __KERNEL__
 
+#include <linux/compiler.h>
+
 extern __inline__ unsigned ld_le16(const volatile unsigned short *addr)
 {
 	unsigned val;
@@ -32,7 +35,7 @@ extern __inline__ void st_le32(volatile 
 	__asm__ __volatile__ ("stwbrx %1,0,%2" : "=m" (*addr) : "r" (val), "r" (addr));
 }
 
-static __inline__ __const__ __u16 ___arch__swab16(__u16 value)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 value)
 {
 	__u16 result;
 
@@ -40,7 +43,7 @@ static __inline__ __const__ __u16 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 value)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 value)
 {
 	__u32 result;
 
diff -puN include/asm-ppc64/byteorder.h~const_fn include/asm-ppc64/byteorder.h
--- i386/include/asm-ppc64/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-ppc64/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -13,6 +13,8 @@
 #ifdef __GNUC__
 #ifdef __KERNEL__
 
+#include <linux/compiler.h>
+
 static __inline__ __u16 ld_le16(const volatile __u16 *addr)
 {
 	__u16 val;
@@ -40,7 +42,7 @@ static __inline__ void st_le32(volatile 
 }
 
 #if 0
-static __inline__ __const__ __u16 ___arch__swab16(__u16 value)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 value)
 {
 	__u16 result;
 
@@ -50,7 +52,7 @@ static __inline__ __const__ __u16 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 value)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 value)
 {
 	__u32 result;
 
@@ -62,7 +64,7 @@ static __inline__ __const__ __u32 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u64 ___arch__swab64(__u64 value)
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 value)
 {
 	__u64 result;
 #error implement me
diff -puN include/asm-s390/byteorder.h~const_fn include/asm-s390/byteorder.h
--- i386/include/asm-s390/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-s390/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -13,8 +13,10 @@
 
 #ifdef __GNUC__
 
+#include <linux/compiler.h>
+
 #ifdef __s390x__
-static __inline__ __const__ __u64 ___arch__swab64p(__u64 *x)
+static __inline__ __attribute_const__ __u64 ___arch__swab64p(__u64 *x)
 {
 	__u64 result;
 
@@ -24,7 +26,7 @@ static __inline__ __const__ __u64 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 x)
 {
 	__u64 result;
 
@@ -40,7 +42,7 @@ static __inline__ void ___arch__swab64s(
 }
 #endif /* __s390x__ */
 
-static __inline__ __const__ __u32 ___arch__swab32p(__u32 *x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32p(__u32 *x)
 {
 	__u32 result;
 	
@@ -58,7 +60,7 @@ static __inline__ __const__ __u32 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
 #ifndef __s390x__
 	return ___arch__swab32p(&x);
@@ -77,7 +79,7 @@ static __inline__ void ___arch__swab32s(
 	*x = ___arch__swab32p(x);
 }
 
-static __inline__ __const__ __u16 ___arch__swab16p(__u16 *x)
+static __inline__ __attribute_const__ __u16 ___arch__swab16p(__u16 *x)
 {
 	__u16 result;
 	
@@ -93,7 +95,7 @@ static __inline__ __const__ __u16 ___arc
 	return result;
 }
 
-static __inline__ __const__ __u16 ___arch__swab16(__u16 x)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
 {
 	return ___arch__swab16p(&x);
 }
diff -puN include/asm-sh/byteorder.h~const_fn include/asm-sh/byteorder.h
--- i386/include/asm-sh/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-sh/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -6,8 +6,9 @@
  */
 
 #include <asm/types.h>
+#include <linux/compiler.h>
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
 	__asm__("swap.b	%0, %0\n\t"
 		"swap.w %0, %0\n\t"
@@ -17,7 +18,7 @@ static __inline__ __const__ __u32 ___arc
 	return x;
 }
 
-static __inline__ __const__ __u16 ___arch__swab16(__u16 x)
+static __inline__ __attribute_const__ __u16 ___arch__swab16(__u16 x)
 {
 	__asm__("swap.b %0, %0"
 		: "=r" (x)
diff -puN include/asm-v850/byteorder.h~const_fn include/asm-v850/byteorder.h
--- i386/include/asm-v850/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-v850/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -18,14 +18,16 @@
 
 #ifdef __GNUC__
 
-static __inline__ __const__ __u32 ___arch__swab32 (__u32 word)
+#include <linux/compiler.h>
+
+static __inline__ __attribute_const__ __u32 ___arch__swab32 (__u32 word)
 {
 	__u32 res;
 	__asm__ ("bsw %1, %0" : "=r" (res) : "r" (word));
 	return res;
 }
 
-static __inline__ __const__ __u16 ___arch__swab16 (__u16 half_word)
+static __inline__ __attribute_const__ __u16 ___arch__swab16 (__u16 half_word)
 {
 	__u16 res;
 	__asm__ ("bsh %1, %0" : "=r" (res) : "r" (half_word));
diff -puN include/asm-x86_64/byteorder.h~const_fn include/asm-x86_64/byteorder.h
--- i386/include/asm-x86_64/byteorder.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/asm-x86_64/byteorder.h	Thu Nov 27 15:22:58 2003
@@ -5,13 +5,15 @@
 
 #ifdef __GNUC__
 
-static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
+#include <linux/compiler.h>
+
+static __inline__ __attribute_const__ __u64 ___arch__swab64(__u64 x)
 {
 	__asm__("bswapq %0" : "=r" (x) : "0" (x));
 	return x;
 }
 
-static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
+static __inline__ __attribute_const__ __u32 ___arch__swab32(__u32 x)
 {
 	__asm__("bswapl %0" : "=r" (x) : "0" (x));
 	return x;
diff -puN include/linux/compiler.h~const_fn include/linux/compiler.h
--- i386/include/linux/compiler.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/compiler.h	Thu Nov 27 15:22:58 2003
@@ -76,6 +76,11 @@
 # define __attribute_pure__	/* unimplemented */
 #endif
 
+#ifndef __attribute_const__
+# define __attribute_const__	/* unimplemented */
+#endif
+
+
 /* Optimization barrier */
 #ifndef barrier
 # define barrier() __memory_barrier()
diff -puN include/linux/byteorder/swab.h~const_fn include/linux/byteorder/swab.h
--- i386/include/linux/byteorder/swab.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/byteorder/swab.h	Thu Nov 27 15:22:58 2003
@@ -1,6 +1,8 @@
 #ifndef _LINUX_BYTEORDER_SWAB_H
 #define _LINUX_BYTEORDER_SWAB_H
 
+#include <linux/compiler.h>
+
 /*
  * linux/byteorder/swab.h
  * Byte-swapping, independently from CPU endianness
@@ -128,7 +130,7 @@
 #endif /* OPTIMIZE */
 
 
-static __inline__ __const__ __u16 __fswab16(__u16 x)
+static __inline__ __attribute_const__ __u16 __fswab16(__u16 x)
 {
 	return __arch__swab16(x);
 }
@@ -141,7 +143,7 @@ static __inline__ void __swab16s(__u16 *
 	__arch__swab16s(addr);
 }
 
-static __inline__ __const__ __u32 __fswab32(__u32 x)
+static __inline__ __attribute_const__ __u32 __fswab32(__u32 x)
 {
 	return __arch__swab32(x);
 }
@@ -155,7 +157,7 @@ static __inline__ void __swab32s(__u32 *
 }
 
 #ifdef __BYTEORDER_HAS_U64__
-static __inline__ __const__ __u64 __fswab64(__u64 x)
+static __inline__ __attribute_const__ __u64 __fswab64(__u64 x)
 {
 #  ifdef __SWAB_64_THRU_32__
 	__u32 h = x >> 32;
diff -puN include/linux/byteorder/swabb.h~const_fn include/linux/byteorder/swabb.h
--- i386/include/linux/byteorder/swabb.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/byteorder/swabb.h	Thu Nov 27 15:22:58 2003
@@ -1,6 +1,8 @@
 #ifndef _LINUX_BYTEORDER_SWABB_H
 #define _LINUX_BYTEORDER_SWABB_H
 
+#include <linux/compiler.h>
+
 /*
  * linux/byteorder/swabb.h
  * SWAp Bytes Bizarrely
@@ -92,7 +94,7 @@
 #endif /* OPTIMIZE */
 
 
-static __inline__ __const__ __u32 __fswahw32(__u32 x)
+static __inline__ __attribute_const__ __u32 __fswahw32(__u32 x)
 {
 	return __arch__swahw32(x);
 }
@@ -106,7 +108,7 @@ static __inline__ void __swahw32s(__u32 
 }
 
 
-static __inline__ __const__ __u32 __fswahb32(__u32 x)
+static __inline__ __attribute_const__ __u32 __fswahb32(__u32 x)
 {
 	return __arch__swahb32(x);
 }
diff -puN include/linux/compiler-gcc2.h~const_fn include/linux/compiler-gcc2.h
--- i386/include/linux/compiler-gcc2.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/compiler-gcc2.h	Thu Nov 27 15:22:58 2003
@@ -21,3 +21,9 @@
 #if __GNUC_MINOR__ >= 96
 # define __attribute_pure__	__attribute__((pure))
 #endif
+
+/* The attribute `const' is not implemented in GCC versions earlier than
+   2.5. We are not interested in elder compilers. */
+/* Basically this is just slightly more strict class than the `pure'
+   attribute */
+#define __attribute_const__ __attribute__ ((__const__))
diff -puN include/linux/compiler-gcc3.h~const_fn include/linux/compiler-gcc3.h
--- i386/include/linux/compiler-gcc3.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/compiler-gcc3.h	Thu Nov 27 15:22:58 2003
@@ -20,3 +20,4 @@
 #endif
 
 #define __attribute_pure__	__attribute__((pure))
+#define __attribute_const__ __attribute__ ((__const__))
diff -puN include/linux/compiler-gcc+.h~const_fn include/linux/compiler-gcc+.h
--- i386/include/linux/compiler-gcc+.h~const_fn	Thu Nov 27 15:22:58 2003
+++ i386-nikita/include/linux/compiler-gcc+.h	Thu Nov 27 15:22:58 

const versus __attribute__((const))

[H. Peter Anvin]

I have been chasing down a bunch of warnings that have been annoying
me, and I have observed that a bunch of the byteorder functions are
defined in ways similar to:

static __inline__ __const__ __u16 ___arch__swab16(__u16 value)

With -W -Wall at least gcc 3.2.2 will issue a warning:

warning: type qualifiers ignored on function return type

... which seems to imply the __const__ is ignored.  Reading the gcc
documentation it appears the correct syntax is
__attribute__((__const__)) rather than __const__.

I have made a patch against the current tree defining
__attribute_const__ in <linux/compiler.h> and using it in the above
cases; does anyone know any reason why I should *NOT* submit this to
Linus?

	-hpa
-- 
<hpa@transmeta.com> at work, <hpa@zytor.com> in private!
If you send me mail in HTML format I will assume it's spam.
"Unix gives you enough rope to shoot yourself in the foot."
Architectures needed: ia64 m68k mips64 ppc ppc64 s390 s390x sh v850 x86-64