| Message ID | 20210817011731.9822-1-sergey.madaminov@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [ovs-dev,v2] include/openvswitch/compiler.h: add check for __builtin_prefetch using __has_builtin | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Mon, Aug 16, 2021 at 08:17:31PM -0500, Sergey Madaminov wrote: > Currently, '__builtin_prefetch' is defined for OVS_PREFETCH macro only > if '__GNUC__' is defined. However, it would make sense to use a > '__has_builtin' preprocessor operator to check if '__builtin_prefetch' > is available and then define the OVS_PREFETCH macro. > > Doing so will allow to use prefetching on Windows too. This operator is > supported by Clang and is supported in GCC starting GCC 10. To preserve > backwards compatibility, the old way to enable prefetching is preserverd > if '__has_builtin' is not defined. > > Also, I've checked the coding style if it has anything related to the > indentations in preprocessor directives but have not found anything. > Checking several files showed that there is no consistent rule to do so. > Thus, I simply followed Google coding style and added indentations > after pound sign for readability. > > Suggested-by: Ben Pfaff <blp@ovn.org> > Signed-off-by: Sergey Madaminov <sergey.madaminov@gmail.com> Hi Sergey, This patch appears to have gone stale in patchwork, for one reason or another. If it is still relevant then I think it needs to be revisited, by being reposted after appropriate preparation. As such I'm marking this patch as "Deferred" in patchwork. No action is required unless there is a desire to revisit this patch.
diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h index cf009f826..3fbf68e9e 100644 --- a/include/openvswitch/compiler.h +++ b/include/openvswitch/compiler.h @@ -26,6 +26,9 @@ #ifndef __has_extension #define __has_extension(x) 0 #endif +#ifndef __has_builtin +# define __has_builtin(x) 0 +#endif /* To make OVS_NO_RETURN portable across gcc/clang and MSVC, it should be * added at the beginning of the function declaration. */ @@ -229,13 +232,17 @@ * OVS_PREFETCH_WRITE() should be used when the memory is going to be * written to. Depending on the target CPU, this can generate the same * instruction as OVS_PREFETCH(), or bring the data into the cache in an - * exclusive state. */ -#if __GNUC__ -#define OVS_PREFETCH(addr) __builtin_prefetch((addr)) -#define OVS_PREFETCH_WRITE(addr) __builtin_prefetch((addr), 1) + * exclusive state. + * + * GCC 10 introduced support for __has_builtin preprocessor operator, + * however, the old way to define OVS_PREFETCH remains to allow for backwards + * compatibility. */ +#if __has_builtin (__builtin_prefetch) || __GNUC__ +# define OVS_PREFETCH(addr) __builtin_prefetch((addr)) +# define OVS_PREFETCH_WRITE(addr) __builtin_prefetch((addr), 1) #else -#define OVS_PREFETCH(addr) -#define OVS_PREFETCH_WRITE(addr) +# define OVS_PREFETCH(addr) +# define OVS_PREFETCH_WRITE(addr) #endif /* Since Visual Studio 2015 there has been an effort to make offsetof a
Currently, '__builtin_prefetch' is defined for OVS_PREFETCH macro only if '__GNUC__' is defined. However, it would make sense to use a '__has_builtin' preprocessor operator to check if '__builtin_prefetch' is available and then define the OVS_PREFETCH macro. Doing so will allow to use prefetching on Windows too. This operator is supported by Clang and is supported in GCC starting GCC 10. To preserve backwards compatibility, the old way to enable prefetching is preserverd if '__has_builtin' is not defined. Also, I've checked the coding style if it has anything related to the indentations in preprocessor directives but have not found anything. Checking several files showed that there is no consistent rule to do so. Thus, I simply followed Google coding style and added indentations after pound sign for readability. Suggested-by: Ben Pfaff <blp@ovn.org> Signed-off-by: Sergey Madaminov <sergey.madaminov@gmail.com> --- v1->v2: addressed Ben's suggestions * added #ifndef for __has_builtin * previously used format for __has_builtin recommended by GCC [1] but that introduced code duplication so addressed that * removed redundant code from patch not related to the matter at hand [1] https://gcc.gnu.org/onlinedocs/gcc-10.1.0/cpp/_005f_005fhas_005fbuiltin.html --- include/openvswitch/compiler.h | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)