Re: use inline functions instead of __statement

2018-01-04 Thread Joerg Sonnenberger
On Thu, Jan 04, 2018 at 09:35:36AM +1000, David Gwynne wrote:
> these days you can use inline functions to get the same effect, but
> it is a more obvious and standard language feature.

If you want to go that way, you still should very likely mark the
functions as always_inline, otherwise the debugging experience will be a
lot more annoying. That said, at least for clang it would be even better
to just use the builtin.

Joerg



Re: use inline functions instead of __statement

2018-01-03 Thread Philip Guenther
On Wed, Jan 3, 2018 at 3:59 PM, Damien Miller  wrote:

> On Thu, 4 Jan 2018, David Gwynne wrote:
>
> > my theory is that __statement (a gcc extension) was used to allow
> > macros to evaluate their argument(s) once by assigning it to a local
> > variable, and then returning a value. this is difficult with normal
> > macros.
>
> Not understanding - doesn't this:
>
> > -#define  __swap32md(x) __statement({
>  \
> > - __uint32_t __swap32md_x = (x);  \
>
> evaluate its argument only once even without __statement?
>

The extension is the "({ ... })" syntax, not the __statement token.  The
latter is only there to suppress "you're using an extension!!!11!!"
warnings.

Without using the gcc ({ ... }) extension, how are you going to create a
scope for a local variable in the middle of an expression?

Philip Guenther


Re: use inline functions instead of __statement

2018-01-03 Thread Damien Miller
On Thu, 4 Jan 2018, David Gwynne wrote:

> my theory is that __statement (a gcc extension) was used to allow
> macros to evaluate their argument(s) once by assigning it to a local
> variable, and then returning a value. this is difficult with normal
> macros.

Not understanding - doesn't this:

> -#define  __swap32md(x) __statement({ 
> \
> - __uint32_t __swap32md_x = (x);  \

evaluate its argument only once even without __statement?

-d



use inline functions instead of __statement

2018-01-03 Thread David Gwynne
my theory is that __statement (a gcc extension) was used to allow
macros to evaluate their argument(s) once by assigning it to a local
variable, and then returning a value. this is difficult with normal
macros.

these days you can use inline functions to get the same effect, but
it is a more obvious and standard language feature.

the last remaining uses of __statement are in some archs endian
implementations.

this changes them to use inline functions. after this we can stop
providing __statement.

tests? ok?

Index: amd64/include/endian.h
===
RCS file: /cvs/src/sys/arch/amd64/include/endian.h,v
retrieving revision 1.6
diff -u -p -r1.6 endian.h
--- amd64/include/endian.h  12 Jul 2014 16:25:08 -  1.6
+++ amd64/include/endian.h  3 Jan 2018 23:23:44 -
@@ -27,33 +27,31 @@
 #ifndef _MACHINE_ENDIAN_H_
 #define _MACHINE_ENDIAN_H_
 
-#ifdef __GNUC__
+#include 
 
-#define__swap32md(x) __statement({ 
\
-   __uint32_t __swap32md_x = (x);  \
-   \
-   __asm ("bswap %0" : "+r" (__swap32md_x));   \
-   __swap32md_x;   \
-})
+static inline __uint32_t
+__swap32md(__uint32_t x)
+{
+   __asm ("bswap %0" : "+r" (x));
+   return (x);
+}
 
-#define__swap64md(x) __statement({ 
\
-   __uint64_t __swap64md_x = (x);  \
-   \
-   __asm ("bswapq %0" : "+r" (__swap64md_x));  \
-   __swap64md_x;   \
-})
+static inline __uint64_t
+__swap64md(__uint64_t x)
+{
+   __asm ("bswapq %0" : "+r" (x));
+   return (x);
+}
 
-#define__swap16md(x) __statement({ 
\
-   __uint16_t __swap16md_x = (x);  \
-   \
-   __asm ("rorw $8, %w0" : "+r" (__swap16md_x));   \
-   __swap16md_x;   \
-})
+static inline __uint16_t
+__swap16md(__uint16_t x)
+{
+   __asm ("rorw $8, %w0" : "+r" (x));
+   return (x);
+}
 
 /* Tell sys/endian.h we have MD variants of the swap macros.  */
 #define __HAVE_MD_SWAP
-
-#endif /* __GNUC__ */
 
 #define _BYTE_ORDER _LITTLE_ENDIAN
 
Index: arm64/include/endian.h
===
RCS file: /cvs/src/sys/arch/arm64/include/endian.h,v
retrieving revision 1.2
diff -u -p -r1.2 endian.h
--- arm64/include/endian.h  6 Feb 2017 04:08:57 -   1.2
+++ arm64/include/endian.h  3 Jan 2018 23:23:44 -
@@ -19,30 +19,34 @@
 #ifndef _MACHINE_ENDIAN_H_
 #define _MACHINE_ENDIAN_H_
 
-#define __swap32md(x) __statement({ \
-__uint32_t __swap32md_x;\
-\
-__asm ("rev %w0, %w1" : "=r" (__swap32md_x) : "r"(x));  \
-__swap32md_x;   \
-})
+#include 
 
-#define __swap64md(x) __statement({ \
-__uint64_t __swap64md_x;   \
-\
-__asm ("rev %x0, %x1" : "=r" (__swap64md_x) : "r"(x));  \
-__swap64md_x;   \
-})
+static inline __uint32_t
+__swap32md(__uint32_t i)
+{
+   __uint32_t o;
+   __asm ("rev %w0, %w1" : "=r" (o) : "r" (i));
+   return (o);
+}
 
-#define __swap16md(x) __statement({ \
-__uint16_t __swap16md_x;   \
-\
-__asm ("rev16 %w0, %w1" : "=r" (__swap16md_x) : "r"(x));\
-__swap16md_x;   \
-})
+static inline __uint64_t
+__swap64md(__uint64_t i)
+{
+   __uint64_t o;
+__asm ("rev %x0, %x1" : "=r" (o) : "r" (i));
+   return (o);
+}
+
+static inline __uint16_t
+__swap16md(__uint16_t i)
+{
+   __uint16_t o;
+__asm ("rev16 %w0, %w1" : "=r" (o) : "r" (i));
+   return (o);
+}
 
 /* Tell sys/endian.h we have MD variants of the swap macros.  */
 #define __HAVE_MD_SWAP
-
 
 #define _BYTE_ORDER _LITTLE_ENDIAN
 #define__STRICT_ALIGNMENT
Index: i386/include/endian.h
===
RCS file: /cvs/src/sys/arch/i386/include/endian.h,v
retrieving revision 1.18
diff -u -p -r1.18