Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Hi!
> > > hm. So if I have
> > >
> > > struct bar {
> > > unsigned long b;
> > > } __attribute__((packed));
> > >
> > > struct foo {
> > > unsigned long u;
> > > struct bar b;
> > > };
> > >
> > > then the compiler can see that foo.b.b is well-aligned, regardless of the
> > > packedness.
> > >
> > > Plus some crazy people compile the kernel with icc (or at least they used
> > > to). What happens there?
> >
> > A quick grep for __attribute__((packed)) and __packed find around 900 hits,
> > I'd probably find more if I'd look for syntactical variations. Some hits
> > are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
> > useful x86 kernel that doesn't involve any packed attribute. I take that
> > as statistical proof that icc either has doesn't really work for building
> > the kernel or groks packing. Any compiler not implementing gcc extensions
> > is lost at building the kernel but that's old news.
> >
>
> No, icc surely supports attribute(packed). My point is that we shouldn't
> rely upon the gcc info file for this, because other compilers can (or
> could) be used to build the kernel.
Well, icc should be gcc compatible. If it is not, it is icc bug.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Hi!
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
Plus some crazy people compile the kernel with icc (or at least they used
to). What happens there?
A quick grep for __attribute__((packed)) and __packed find around 900 hits,
I'd probably find more if I'd look for syntactical variations. Some hits
are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
useful x86 kernel that doesn't involve any packed attribute. I take that
as statistical proof that icc either has doesn't really work for building
the kernel or groks packing. Any compiler not implementing gcc extensions
is lost at building the kernel but that's old news.
No, icc surely supports attribute(packed). My point is that we shouldn't
rely upon the gcc info file for this, because other compilers can (or
could) be used to build the kernel.
Well, icc should be gcc compatible. If it is not, it is icc bug.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 05:27:20PM -0800, Andrew Morton wrote: > No, icc surely supports attribute(packed). My point is that we shouldn't > rely upon the gcc info file for this, because other compilers can (or > could) be used to build the kernel. > > So it would be safer if the C spec said (or could be interpreted to say) > "members of packed structures are always copied bytewise". So then we > can be reasonably confident that this change won't break the use of > those compilers. > > But then, I don't even know if any C standard says anything about packing. Memory layout and alignment of structures and members are implementation defined according to the C standard; the standard provides no means to influence these. So it takes a compiler extension such as gcc's __attribute__(). > Ho hum. Why are we talking about this, anyway? Does the patch make the > code faster? Or just nicer? Smaller binary and from looking at the disassembly a tad faster also. Ralf - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Fri, 16 Feb 2007 00:43:17 +
Ralf Baechle <[EMAIL PROTECTED]> wrote:
> On Thu, Feb 15, 2007 at 03:38:23PM -0800, Andrew Morton wrote:
>
> > hm. So if I have
> >
> > struct bar {
> > unsigned long b;
> > } __attribute__((packed));
> >
> > struct foo {
> > unsigned long u;
> > struct bar b;
> > };
> >
> > then the compiler can see that foo.b.b is well-aligned, regardless of the
> > packedness.
> >
> > Plus some crazy people compile the kernel with icc (or at least they used
> > to). What happens there?
>
> A quick grep for __attribute__((packed)) and __packed find around 900 hits,
> I'd probably find more if I'd look for syntactical variations. Some hits
> are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
> useful x86 kernel that doesn't involve any packed attribute. I take that
> as statistical proof that icc either has doesn't really work for building
> the kernel or groks packing. Any compiler not implementing gcc extensions
> is lost at building the kernel but that's old news.
>
No, icc surely supports attribute(packed). My point is that we shouldn't
rely upon the gcc info file for this, because other compilers can (or
could) be used to build the kernel.
So it would be safer if the C spec said (or could be interpreted to say)
"members of packed structures are always copied bytewise". So then we
can be reasonably confident that this change won't break the use of
those compilers.
But then, I don't even know if any C standard says anything about packing.
Ho hum. Why are we talking about this, anyway? Does the patch make the
code faster? Or just nicer?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 03:38:23PM -0800, Andrew Morton wrote:
> hm. So if I have
>
> struct bar {
> unsigned long b;
> } __attribute__((packed));
>
> struct foo {
> unsigned long u;
> struct bar b;
> };
>
> then the compiler can see that foo.b.b is well-aligned, regardless of the
> packedness.
>
> Plus some crazy people compile the kernel with icc (or at least they used
> to). What happens there?
A quick grep for __attribute__((packed)) and __packed find around 900 hits,
I'd probably find more if I'd look for syntactical variations. Some hits
are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
useful x86 kernel that doesn't involve any packed attribute. I take that
as statistical proof that icc either has doesn't really work for building
the kernel or groks packing. Any compiler not implementing gcc extensions
is lost at building the kernel but that's old news.
Ralf
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Andrew Morton wrote:
> hm. So if I have
>
> struct bar {
> unsigned long b;
> } __attribute__((packed));
>
> struct foo {
> unsigned long u;
> struct bar b;
> };
>
> then the compiler can see that foo.b.b is well-aligned, regardless of the
> packedness.
In Ralf's code, the structure is anonymous, and is used to declare a
pointer type, which is initialized from a void *. So I think the
compiler isn't allowed to assume anything about its alignment.
J
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, 15 Feb 2007 22:18:39 +
Ralf Baechle <[EMAIL PROTECTED]> wrote:
> On Thu, Feb 15, 2007 at 01:53:58PM -0800, Andrew Morton wrote:
>
> > > The whole union thing was only needed to get rid of a warning but Marcel's
> > > solution does the same thing by attaching the packed keyword to the entire
> > > structure instead, so this patch is now using his macros but using
> > > __packed
> > > instead.
> >
> > How do we know this trick will work as-designed across all versions of gcc
> > and icc (at least) and for all architectures and for all sets of compiler
> > options?
> >
> > Basically, it has to be guaranteed by a C standard. Is it?
>
> Gcc info page says:
>
> [...]
> `packed'
> The `packed' attribute specifies that a variable or structure field
> should have the smallest possible alignment--one byte for a
> variable, and one bit for a field, unless you specify a larger
> value with the `aligned' attribute.
> [...]
>
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
Plus some crazy people compile the kernel with icc (or at least they used
to). What happens there?
> Qed?
worried.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Ralf Baechle wrote: > Gcc info page says: > > [...] > `packed' > The `packed' attribute specifies that a variable or structure field > should have the smallest possible alignment--one byte for a > variable, and one bit for a field, unless you specify a larger > value with the `aligned' attribute. > [...] > > Qed? So that the compiler has to assume that if its accessing this __packed structure, it may be embedded unaligned within something else? And because the pointer is cast through (void *) it isn't allowed to use alias analysis to notice that the pointer wasn't originally (apparently) unaligned. Seems sound to me. J - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 01:53:58PM -0800, Andrew Morton wrote: > > The whole union thing was only needed to get rid of a warning but Marcel's > > solution does the same thing by attaching the packed keyword to the entire > > structure instead, so this patch is now using his macros but using __packed > > instead. > > How do we know this trick will work as-designed across all versions of gcc > and icc (at least) and for all architectures and for all sets of compiler > options? > > Basically, it has to be guaranteed by a C standard. Is it? Gcc info page says: [...] `packed' The `packed' attribute specifies that a variable or structure field should have the smallest possible alignment--one byte for a variable, and one bit for a field, unless you specify a larger value with the `aligned' attribute. [...] Qed? Ralf - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, 15 Feb 2007 14:34:41 + Ralf Baechle <[EMAIL PROTECTED]> wrote: > On Wed, Feb 14, 2007 at 08:39:03PM -0800, Andrew Morton wrote: > > > Can someone please tell us how this magic works? (And it does appear to > > work). > > > > It seems to assuming that the compiler will assume that members of packed > > structures can have arbitrary alignment, even if that alignment is obvious. > > > > Which makes sense, but I'd like to see chapter-and-verse from the spec or > > from the gcc docs so we can rely upon it working on all architectures and > > compilers from now until ever more. > > > > IOW: your changlogging sucks ;) > > It was my entry for the next edition of the C Puzzle Book ;-) > > The whole union thing was only needed to get rid of a warning but Marcel's > solution does the same thing by attaching the packed keyword to the entire > structure instead, so this patch is now using his macros but using __packed > instead. How do we know this trick will work as-designed across all versions of gcc and icc (at least) and for all architectures and for all sets of compiler options? Basically, it has to be guaranteed by a C standard. Is it? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Wed, Feb 14, 2007 at 08:39:03PM -0800, Andrew Morton wrote:
> Can someone please tell us how this magic works? (And it does appear to
> work).
>
> It seems to assuming that the compiler will assume that members of packed
> structures can have arbitrary alignment, even if that alignment is obvious.
>
> Which makes sense, but I'd like to see chapter-and-verse from the spec or
> from the gcc docs so we can rely upon it working on all architectures and
> compilers from now until ever more.
>
> IOW: your changlogging sucks ;)
It was my entry for the next edition of the C Puzzle Book ;-)
The whole union thing was only needed to get rid of a warning but Marcel's
solution does the same thing by attaching the packed keyword to the entire
structure instead, so this patch is now using his macros but using __packed
instead.
Ralf
Signed-off-by: Ralf Baechle <[EMAIL PROTECTED]>
diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 09ec447..60d94fc 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -1,122 +1,27 @@
-#ifndef _ASM_GENERIC_UNALIGNED_H_
-#define _ASM_GENERIC_UNALIGNED_H_
-
-/*
- * For the benefit of those who are trying to port Linux to another
- * architecture, here are some C-language equivalents.
- *
- * This is based almost entirely upon Richard Henderson's
- * asm-alpha/unaligned.h implementation. Some comments were
- * taken from David Mosberger's asm-ia64/unaligned.h header.
- */
-
-#include
-
-/*
- * The main single-value unaligned transfer routines.
- */
-#define get_unaligned(ptr) \
- __get_unaligned((ptr), sizeof(*(ptr)))
-#define put_unaligned(x,ptr) \
- __put_unaligned((__u64)(x), (ptr), sizeof(*(ptr)))
-
/*
- * This function doesn't actually exist. The idea is that when
- * someone uses the macros below with an unsupported size (datatype),
- * the linker will alert us to the problem via an unresolved reference
- * error.
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
*/
-extern void bad_unaligned_access_length(void) __attribute__((noreturn));
-
-struct __una_u64 { __u64 x __attribute__((packed)); };
-struct __una_u32 { __u32 x __attribute__((packed)); };
-struct __una_u16 { __u16 x __attribute__((packed)); };
-
-/*
- * Elemental unaligned loads
- */
-
-static inline __u64 __uldq(const __u64 *addr)
-{
- const struct __una_u64 *ptr = (const struct __una_u64 *) addr;
- return ptr->x;
-}
-
-static inline __u32 __uldl(const __u32 *addr)
-{
- const struct __una_u32 *ptr = (const struct __una_u32 *) addr;
- return ptr->x;
-}
-
-static inline __u16 __uldw(const __u16 *addr)
-{
- const struct __una_u16 *ptr = (const struct __una_u16 *) addr;
- return ptr->x;
-}
-
-/*
- * Elemental unaligned stores
- */
-
-static inline void __ustq(__u64 val, __u64 *addr)
-{
- struct __una_u64 *ptr = (struct __una_u64 *) addr;
- ptr->x = val;
-}
-
-static inline void __ustl(__u32 val, __u32 *addr)
-{
- struct __una_u32 *ptr = (struct __una_u32 *) addr;
- ptr->x = val;
-}
+#ifndef __ASM_GENERIC_UNALIGNED_H
+#define __ASM_GENERIC_UNALIGNED_H
-static inline void __ustw(__u16 val, __u16 *addr)
-{
- struct __una_u16 *ptr = (struct __una_u16 *) addr;
- ptr->x = val;
-}
+#include
-#define __get_unaligned(ptr, size) ({ \
- const void *__gu_p = ptr; \
- __u64 val; \
- switch (size) { \
- case 1: \
- val = *(const __u8 *)__gu_p;\
- break; \
- case 2: \
- val = __uldw(__gu_p); \
- break; \
- case 4: \
- val = __uldl(__gu_p); \
- break; \
- case 8: \
- val = __uldq(__gu_p); \
- break; \
- default:\
- bad_unaligned_access_length(); \
- }; \
- (__typeof__(*(ptr)))val;\
+#define get_unaligned(ptr) \
+({ \
+ struct __packed { \
+ typeof(*(ptr)) __v; \
+ } *__p = (void *) (ptr);\
+ __p->__v; \
})
-#define __put_unaligned(val, ptr, size)\
-do { \
- void *__gu_p = ptr; \
- switch
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Hi Andrew,
> > +#define get_unaligned(ptr) \
> > +({ \
> > + const struct { \
> > + union { \
> > + const int __un_foo[0]; \
> > + const __typeof__(*(ptr)) __un; \
> > + } __un __attribute__ ((packed));\
> > + } * const __gu_p = (void *) (ptr); \
> > + \
> > + __gu_p->__un.__un; \
> > })
>
> Can someone please tell us how this magic works? (And it does appear to
> work).
>
> It seems to assuming that the compiler will assume that members of packed
> structures can have arbitrary alignment, even if that alignment is obvious.
>
> Which makes sense, but I'd like to see chapter-and-verse from the spec or
> from the gcc docs so we can rely upon it working on all architectures and
> compilers from now until ever more.
I am far away from having any knowledge about the GCC internals and the
reason why this code works, but I've been told the generic way of
handling unaligned access is this:
#define get_unaligned(ptr) \
({ \
struct __attribute__((packed)) {\
typeof(*(ptr)) __v; \
} *__p = (void *) (ptr);\
__p->__v; \
})
#define put_unaligned(val, ptr) \
do {\
struct __attribute__((packed)) {\
typeof(*(ptr)) __v; \
} *__p = (void *) (ptr);\
__p->__v = (val); \
} while(0)
Actually I am using this code in the Bluetooth userspace library for
over two years now without any complaints.
Regards
Marcel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Hi Andrew,
+#define get_unaligned(ptr) \
+({ \
+ const struct { \
+ union { \
+ const int __un_foo[0]; \
+ const __typeof__(*(ptr)) __un; \
+ } __un __attribute__ ((packed));\
+ } * const __gu_p = (void *) (ptr); \
+ \
+ __gu_p-__un.__un; \
})
Can someone please tell us how this magic works? (And it does appear to
work).
It seems to assuming that the compiler will assume that members of packed
structures can have arbitrary alignment, even if that alignment is obvious.
Which makes sense, but I'd like to see chapter-and-verse from the spec or
from the gcc docs so we can rely upon it working on all architectures and
compilers from now until ever more.
I am far away from having any knowledge about the GCC internals and the
reason why this code works, but I've been told the generic way of
handling unaligned access is this:
#define get_unaligned(ptr) \
({ \
struct __attribute__((packed)) {\
typeof(*(ptr)) __v; \
} *__p = (void *) (ptr);\
__p-__v; \
})
#define put_unaligned(val, ptr) \
do {\
struct __attribute__((packed)) {\
typeof(*(ptr)) __v; \
} *__p = (void *) (ptr);\
__p-__v = (val); \
} while(0)
Actually I am using this code in the Bluetooth userspace library for
over two years now without any complaints.
Regards
Marcel
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Wed, Feb 14, 2007 at 08:39:03PM -0800, Andrew Morton wrote:
Can someone please tell us how this magic works? (And it does appear to
work).
It seems to assuming that the compiler will assume that members of packed
structures can have arbitrary alignment, even if that alignment is obvious.
Which makes sense, but I'd like to see chapter-and-verse from the spec or
from the gcc docs so we can rely upon it working on all architectures and
compilers from now until ever more.
IOW: your changlogging sucks ;)
It was my entry for the next edition of the C Puzzle Book ;-)
The whole union thing was only needed to get rid of a warning but Marcel's
solution does the same thing by attaching the packed keyword to the entire
structure instead, so this patch is now using his macros but using __packed
instead.
Ralf
Signed-off-by: Ralf Baechle [EMAIL PROTECTED]
diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h
index 09ec447..60d94fc 100644
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -1,122 +1,27 @@
-#ifndef _ASM_GENERIC_UNALIGNED_H_
-#define _ASM_GENERIC_UNALIGNED_H_
-
-/*
- * For the benefit of those who are trying to port Linux to another
- * architecture, here are some C-language equivalents.
- *
- * This is based almost entirely upon Richard Henderson's
- * asm-alpha/unaligned.h implementation. Some comments were
- * taken from David Mosberger's asm-ia64/unaligned.h header.
- */
-
-#include linux/types.h
-
-/*
- * The main single-value unaligned transfer routines.
- */
-#define get_unaligned(ptr) \
- __get_unaligned((ptr), sizeof(*(ptr)))
-#define put_unaligned(x,ptr) \
- __put_unaligned((__u64)(x), (ptr), sizeof(*(ptr)))
-
/*
- * This function doesn't actually exist. The idea is that when
- * someone uses the macros below with an unsupported size (datatype),
- * the linker will alert us to the problem via an unresolved reference
- * error.
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file COPYING in the main directory of this archive
+ * for more details.
*/
-extern void bad_unaligned_access_length(void) __attribute__((noreturn));
-
-struct __una_u64 { __u64 x __attribute__((packed)); };
-struct __una_u32 { __u32 x __attribute__((packed)); };
-struct __una_u16 { __u16 x __attribute__((packed)); };
-
-/*
- * Elemental unaligned loads
- */
-
-static inline __u64 __uldq(const __u64 *addr)
-{
- const struct __una_u64 *ptr = (const struct __una_u64 *) addr;
- return ptr-x;
-}
-
-static inline __u32 __uldl(const __u32 *addr)
-{
- const struct __una_u32 *ptr = (const struct __una_u32 *) addr;
- return ptr-x;
-}
-
-static inline __u16 __uldw(const __u16 *addr)
-{
- const struct __una_u16 *ptr = (const struct __una_u16 *) addr;
- return ptr-x;
-}
-
-/*
- * Elemental unaligned stores
- */
-
-static inline void __ustq(__u64 val, __u64 *addr)
-{
- struct __una_u64 *ptr = (struct __una_u64 *) addr;
- ptr-x = val;
-}
-
-static inline void __ustl(__u32 val, __u32 *addr)
-{
- struct __una_u32 *ptr = (struct __una_u32 *) addr;
- ptr-x = val;
-}
+#ifndef __ASM_GENERIC_UNALIGNED_H
+#define __ASM_GENERIC_UNALIGNED_H
-static inline void __ustw(__u16 val, __u16 *addr)
-{
- struct __una_u16 *ptr = (struct __una_u16 *) addr;
- ptr-x = val;
-}
+#include linux/compiler.h
-#define __get_unaligned(ptr, size) ({ \
- const void *__gu_p = ptr; \
- __u64 val; \
- switch (size) { \
- case 1: \
- val = *(const __u8 *)__gu_p;\
- break; \
- case 2: \
- val = __uldw(__gu_p); \
- break; \
- case 4: \
- val = __uldl(__gu_p); \
- break; \
- case 8: \
- val = __uldq(__gu_p); \
- break; \
- default:\
- bad_unaligned_access_length(); \
- }; \
- (__typeof__(*(ptr)))val;\
+#define get_unaligned(ptr) \
+({ \
+ struct __packed { \
+ typeof(*(ptr)) __v; \
+ } *__p = (void *) (ptr);\
+ __p-__v; \
})
-#define __put_unaligned(val, ptr, size)\
-do { \
- void *__gu_p = ptr; \
-
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, 15 Feb 2007 14:34:41 + Ralf Baechle [EMAIL PROTECTED] wrote: On Wed, Feb 14, 2007 at 08:39:03PM -0800, Andrew Morton wrote: Can someone please tell us how this magic works? (And it does appear to work). It seems to assuming that the compiler will assume that members of packed structures can have arbitrary alignment, even if that alignment is obvious. Which makes sense, but I'd like to see chapter-and-verse from the spec or from the gcc docs so we can rely upon it working on all architectures and compilers from now until ever more. IOW: your changlogging sucks ;) It was my entry for the next edition of the C Puzzle Book ;-) The whole union thing was only needed to get rid of a warning but Marcel's solution does the same thing by attaching the packed keyword to the entire structure instead, so this patch is now using his macros but using __packed instead. How do we know this trick will work as-designed across all versions of gcc and icc (at least) and for all architectures and for all sets of compiler options? Basically, it has to be guaranteed by a C standard. Is it? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 01:53:58PM -0800, Andrew Morton wrote: The whole union thing was only needed to get rid of a warning but Marcel's solution does the same thing by attaching the packed keyword to the entire structure instead, so this patch is now using his macros but using __packed instead. How do we know this trick will work as-designed across all versions of gcc and icc (at least) and for all architectures and for all sets of compiler options? Basically, it has to be guaranteed by a C standard. Is it? Gcc info page says: [...] `packed' The `packed' attribute specifies that a variable or structure field should have the smallest possible alignment--one byte for a variable, and one bit for a field, unless you specify a larger value with the `aligned' attribute. [...] Qed? Ralf - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Ralf Baechle wrote: Gcc info page says: [...] `packed' The `packed' attribute specifies that a variable or structure field should have the smallest possible alignment--one byte for a variable, and one bit for a field, unless you specify a larger value with the `aligned' attribute. [...] Qed? So that the compiler has to assume that if its accessing this __packed structure, it may be embedded unaligned within something else? And because the pointer is cast through (void *) it isn't allowed to use alias analysis to notice that the pointer wasn't originally (apparently) unaligned. Seems sound to me. J - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, 15 Feb 2007 22:18:39 +
Ralf Baechle [EMAIL PROTECTED] wrote:
On Thu, Feb 15, 2007 at 01:53:58PM -0800, Andrew Morton wrote:
The whole union thing was only needed to get rid of a warning but Marcel's
solution does the same thing by attaching the packed keyword to the entire
structure instead, so this patch is now using his macros but using
__packed
instead.
How do we know this trick will work as-designed across all versions of gcc
and icc (at least) and for all architectures and for all sets of compiler
options?
Basically, it has to be guaranteed by a C standard. Is it?
Gcc info page says:
[...]
`packed'
The `packed' attribute specifies that a variable or structure field
should have the smallest possible alignment--one byte for a
variable, and one bit for a field, unless you specify a larger
value with the `aligned' attribute.
[...]
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
Plus some crazy people compile the kernel with icc (or at least they used
to). What happens there?
Qed?
worried.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
Andrew Morton wrote:
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
In Ralf's code, the structure is anonymous, and is used to declare a
pointer type, which is initialized from a void *. So I think the
compiler isn't allowed to assume anything about its alignment.
J
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 03:38:23PM -0800, Andrew Morton wrote:
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
Plus some crazy people compile the kernel with icc (or at least they used
to). What happens there?
A quick grep for __attribute__((packed)) and __packed find around 900 hits,
I'd probably find more if I'd look for syntactical variations. Some hits
are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
useful x86 kernel that doesn't involve any packed attribute. I take that
as statistical proof that icc either has doesn't really work for building
the kernel or groks packing. Any compiler not implementing gcc extensions
is lost at building the kernel but that's old news.
Ralf
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Fri, 16 Feb 2007 00:43:17 +
Ralf Baechle [EMAIL PROTECTED] wrote:
On Thu, Feb 15, 2007 at 03:38:23PM -0800, Andrew Morton wrote:
hm. So if I have
struct bar {
unsigned long b;
} __attribute__((packed));
struct foo {
unsigned long u;
struct bar b;
};
then the compiler can see that foo.b.b is well-aligned, regardless of the
packedness.
Plus some crazy people compile the kernel with icc (or at least they used
to). What happens there?
A quick grep for __attribute__((packed)) and __packed find around 900 hits,
I'd probably find more if I'd look for syntactical variations. Some hits
are in arch/{i386,x86_64,ia64}. At a glance it seems hard to configure a
useful x86 kernel that doesn't involve any packed attribute. I take that
as statistical proof that icc either has doesn't really work for building
the kernel or groks packing. Any compiler not implementing gcc extensions
is lost at building the kernel but that's old news.
No, icc surely supports attribute(packed). My point is that we shouldn't
rely upon the gcc info file for this, because other compilers can (or
could) be used to build the kernel.
So it would be safer if the C spec said (or could be interpreted to say)
members of packed structures are always copied bytewise. So then we
can be reasonably confident that this change won't break the use of
those compilers.
But then, I don't even know if any C standard says anything about packing.
Ho hum. Why are we talking about this, anyway? Does the patch make the
code faster? Or just nicer?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Thu, Feb 15, 2007 at 05:27:20PM -0800, Andrew Morton wrote: No, icc surely supports attribute(packed). My point is that we shouldn't rely upon the gcc info file for this, because other compilers can (or could) be used to build the kernel. So it would be safer if the C spec said (or could be interpreted to say) members of packed structures are always copied bytewise. So then we can be reasonably confident that this change won't break the use of those compilers. But then, I don't even know if any C standard says anything about packing. Memory layout and alignment of structures and members are implementation defined according to the C standard; the standard provides no means to influence these. So it takes a compiler extension such as gcc's __attribute__(). Ho hum. Why are we talking about this, anyway? Does the patch make the code faster? Or just nicer? Smaller binary and from looking at the disassembly a tad faster also. Ralf - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Wed, 14 Feb 2007 21:42:26 + Ralf Baechle <[EMAIL PROTECTED]> wrote:
> Time for a little bit of dead horse flogging.
>
> On Mon, Mar 06, 2006 at 05:05:52PM -0800, Andrew Morton wrote:
>
> > > --- a/include/asm-generic/unaligned.h
> > > +++ b/include/asm-generic/unaligned.h
> > > @@ -78,7 +78,7 @@ static inline void __ustw(__u16 val, __u
> > >
> > > #define __get_unaligned(ptr, size) ({\
> > > const void *__gu_p = ptr; \
> > > - __typeof__(*(ptr)) val; \
> > > + __u64 val; \
> > > switch (size) { \
> > > case 1: \
> > > val = *(const __u8 *)__gu_p;\
> > > @@ -95,7 +95,7 @@ static inline void __ustw(__u16 val, __u
> > > default:\
> > > bad_unaligned_access_length(); \
> > > }; \
> > > - val;\
> > > + (__typeof__(*(ptr)))val;\
> > > })
> > >
> > > #define __put_unaligned(val, ptr, size) \
> >
> > I worry about what impact that change might have on code generation.
> > Hopefully none, if gcc is good enough.
> >
> > But I cannot think of a better fix.
>
> It does inflate the code but back then we agreed to go for Atsushi's patch
> because it was fairly obviously correct. This patch obviously is less
> obvious but generates fairly decent, works for arbitrary data types and
> cuts down the size of unaligned.h from 122 lines to 44 so it must be good.
>
> ...
>
> +#define get_unaligned(ptr) \
> +({ \
> + const struct { \
> + union { \
> + const int __un_foo[0]; \
> + const __typeof__(*(ptr)) __un; \
> + } __un __attribute__ ((packed));\
> + } * const __gu_p = (void *) (ptr); \
> + \
> + __gu_p->__un.__un; \
> })
Can someone please tell us how this magic works? (And it does appear to
work).
It seems to assuming that the compiler will assume that members of packed
structures can have arbitrary alignment, even if that alignment is obvious.
Which makes sense, but I'd like to see chapter-and-verse from the spec or
from the gcc docs so we can rely upon it working on all architectures and
compilers from now until ever more.
IOW: your changlogging sucks ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Optimize generic get_unaligned / put_unaligned implementations.
On Wed, 14 Feb 2007 21:42:26 + Ralf Baechle [EMAIL PROTECTED] wrote:
Time for a little bit of dead horse flogging.
On Mon, Mar 06, 2006 at 05:05:52PM -0800, Andrew Morton wrote:
--- a/include/asm-generic/unaligned.h
+++ b/include/asm-generic/unaligned.h
@@ -78,7 +78,7 @@ static inline void __ustw(__u16 val, __u
#define __get_unaligned(ptr, size) ({\
const void *__gu_p = ptr; \
- __typeof__(*(ptr)) val; \
+ __u64 val; \
switch (size) { \
case 1: \
val = *(const __u8 *)__gu_p;\
@@ -95,7 +95,7 @@ static inline void __ustw(__u16 val, __u
default:\
bad_unaligned_access_length(); \
}; \
- val;\
+ (__typeof__(*(ptr)))val;\
})
#define __put_unaligned(val, ptr, size) \
I worry about what impact that change might have on code generation.
Hopefully none, if gcc is good enough.
But I cannot think of a better fix.
It does inflate the code but back then we agreed to go for Atsushi's patch
because it was fairly obviously correct. This patch obviously is less
obvious but generates fairly decent, works for arbitrary data types and
cuts down the size of unaligned.h from 122 lines to 44 so it must be good.
...
+#define get_unaligned(ptr) \
+({ \
+ const struct { \
+ union { \
+ const int __un_foo[0]; \
+ const __typeof__(*(ptr)) __un; \
+ } __un __attribute__ ((packed));\
+ } * const __gu_p = (void *) (ptr); \
+ \
+ __gu_p-__un.__un; \
})
Can someone please tell us how this magic works? (And it does appear to
work).
It seems to assuming that the compiler will assume that members of packed
structures can have arbitrary alignment, even if that alignment is obvious.
Which makes sense, but I'd like to see chapter-and-verse from the spec or
from the gcc docs so we can rely upon it working on all architectures and
compilers from now until ever more.
IOW: your changlogging sucks ;)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
