[PATCH] remove wrappers for GCC < 4.8

[PATCH] remove wrappers for GCC < 4.8

[David Marchand]

As we made our move to C11, GCC requirements has been bumped.
Wrappers for old versions of GCC can be dropped.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/eal/arm/include/rte_byteorder.h     | 19 -------------------
 lib/eal/arm/include/rte_vect.h          | 12 ------------
 lib/eal/include/generic/rte_byteorder.h |  2 --
 lib/eal/include/rte_debug.h             |  4 ----
 lib/eal/ppc/include/rte_byteorder.h     |  9 ---------
 lib/eal/x86/include/rte_byteorder.h     |  8 --------
 lib/eal/x86/include/rte_vect.h          |  9 +--------
 lib/log/rte_log.h                       |  4 ----
 8 files changed, 1 insertion(+), 66 deletions(-)

diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h
index df2f1d87ba..ff02052f2e 100644
--- a/lib/eal/arm/include/rte_byteorder.h
+++ b/lib/eal/arm/include/rte_byteorder.h
@@ -17,25 +17,6 @@ extern "C" {
 #include <rte_common.h>
 #include "generic/rte_byteorder.h"
 
-/* fix missing __builtin_bswap16 for gcc older then 4.8 */
-#if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8))
-
-static inline uint16_t rte_arch_bswap16(uint16_t _x)
-{
-	uint16_t x = _x;
-
-	asm volatile ("rev16 %w0,%w1"
-		      : "=r" (x)
-		      : "r" (x)
-		      );
-	return x;
-}
-
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ? \
-				   rte_constant_bswap16(x) : \
-				   rte_arch_bswap16(x)))
-#endif
-
 /* ARM architecture is bi-endian (both big and little). */
 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 
diff --git a/lib/eal/arm/include/rte_vect.h b/lib/eal/arm/include/rte_vect.h
index 4b705bac5f..8cfe4bddc1 100644
--- a/lib/eal/arm/include/rte_vect.h
+++ b/lib/eal/arm/include/rte_vect.h
@@ -83,18 +83,6 @@ vcopyq_laneq_u32(uint32x4_t a, const int lane_a,
 #if defined(RTE_ARCH_ARM64)
 #if RTE_CC_IS_GNU && (GCC_VERSION < 70000)
 
-#if (GCC_VERSION < 40900)
-typedef uint64_t poly64_t;
-typedef uint64x2_t poly64x2_t;
-typedef uint8_t poly128_t __attribute__((vector_size(16), aligned(16)));
-
-static inline uint32x4_t
-vceqzq_u32(uint32x4_t a)
-{
-	return (a == 0);
-}
-#endif
-
 /* NEON intrinsic vreinterpretq_u64_p128() is supported since GCC version 7 */
 static inline uint64x2_t
 vreinterpretq_u64_p128(poly128_t x)
diff --git a/lib/eal/include/generic/rte_byteorder.h b/lib/eal/include/generic/rte_byteorder.h
index a67e1d70d9..03e9a24718 100644
--- a/lib/eal/include/generic/rte_byteorder.h
+++ b/lib/eal/include/generic/rte_byteorder.h
@@ -234,9 +234,7 @@ static uint64_t rte_be_to_cpu_64(rte_be64_t x);
 #endif /* __DOXYGEN__ */
 
 #ifdef RTE_FORCE_INTRINSICS
-#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
 #define rte_bswap16(x) __builtin_bswap16(x)
-#endif
 
 #define rte_bswap32(x) __builtin_bswap32(x)
 
diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
index 2c4b94a7c9..74593cd4d4 100644
--- a/lib/eal/include/rte_debug.h
+++ b/lib/eal/include/rte_debug.h
@@ -60,11 +60,7 @@ void rte_dump_stack(void);
  * documentation.
  */
 void __rte_panic(const char *funcname , const char *format, ...)
-#ifdef __GNUC__
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
 	__rte_cold
-#endif
-#endif
 	__rte_noreturn
 	__rte_format_printf(2, 3);
 
diff --git a/lib/eal/ppc/include/rte_byteorder.h b/lib/eal/ppc/include/rte_byteorder.h
index 49f369f1e7..de94e2ad32 100644
--- a/lib/eal/ppc/include/rte_byteorder.h
+++ b/lib/eal/ppc/include/rte_byteorder.h
@@ -62,15 +62,6 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
 #define rte_bswap64(x) ((uint64_t)(__builtin_constant_p(x) ?		\
 				   rte_constant_bswap64(x) :		\
 				   rte_arch_bswap64(x)))
-#else
-/*
- * __builtin_bswap16 is only available gcc 4.8 and upwards
- */
-#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
-				   rte_constant_bswap16(x) :		\
-				   rte_arch_bswap16(x)))
-#endif
 #endif
 
 /* Power 8 have both little endian and big endian mode
diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
index a2dfecc1f5..57253d356a 100644
--- a/lib/eal/x86/include/rte_byteorder.h
+++ b/lib/eal/x86/include/rte_byteorder.h
@@ -60,14 +60,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 				   rte_constant_bswap64(x) :		\
 				   rte_arch_bswap64(x)))
 #else
-/*
- * __builtin_bswap16 is only available gcc 4.8 and upwards
- */
-#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
-				   rte_constant_bswap16(x) :		\
-				   rte_arch_bswap16(x)))
-#endif
 #endif
 
 #define rte_cpu_to_le_16(x) (x)
diff --git a/lib/eal/x86/include/rte_vect.h b/lib/eal/x86/include/rte_vect.h
index 2e40b77da9..560f9e4db3 100644
--- a/lib/eal/x86/include/rte_vect.h
+++ b/lib/eal/x86/include/rte_vect.h
@@ -16,20 +16,13 @@
 #include <rte_common.h>
 #include "generic/rte_vect.h"
 
-#if (defined(__ICC) || \
-	(defined(_WIN64)) || \
-	(__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
-
+#if defined(__ICC) || defined(_WIN64)
 #include <smmintrin.h> /* SSE4 */
-
 #if defined(__AVX__)
 #include <immintrin.h>
 #endif
-
 #else
-
 #include <x86intrin.h>
-
 #endif
 
 #ifdef __cplusplus
diff --git a/lib/log/rte_log.h b/lib/log/rte_log.h
index bdefff2a59..f7a8405de9 100644
--- a/lib/log/rte_log.h
+++ b/lib/log/rte_log.h
@@ -277,11 +277,7 @@ void rte_log_dump(FILE *f);
  *   - Negative on error.
  */
 int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
-#ifdef __GNUC__
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
 	__rte_cold
-#endif
-#endif
 	__rte_format_printf(3, 4);
 
 /**
-- 
2.41.0

Re: [PATCH] remove wrappers for GCC < 4.8

[David Marchand]

Hello Tyler,

On Thu, Aug 24, 2023 at 10:30 AM David Marchand
<david.marchand@redhat.com> wrote:
> diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
> index 2c4b94a7c9..74593cd4d4 100644
> --- a/lib/eal/include/rte_debug.h
> +++ b/lib/eal/include/rte_debug.h
> @@ -60,11 +60,7 @@ void rte_dump_stack(void);
>   * documentation.
>   */
>  void __rte_panic(const char *funcname , const char *format, ...)
> -#ifdef __GNUC__
> -#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
>         __rte_cold
> -#endif
> -#endif

I don't see some wrapping around __rte_cold for MSVC in your series.
Would this patch break MSVC buidlds?

>         __rte_noreturn
>         __rte_format_printf(2, 3);
>


-- 
David Marchand

Re: [PATCH] remove wrappers for GCC < 4.8

[Bruce Richardson]

On Thu, Aug 24, 2023 at 10:30:34AM +0200, David Marchand wrote:
> As we made our move to C11, GCC requirements has been bumped.
> Wrappers for old versions of GCC can be dropped.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/eal/arm/include/rte_byteorder.h     | 19 -------------------
>  lib/eal/arm/include/rte_vect.h          | 12 ------------
>  lib/eal/include/generic/rte_byteorder.h |  2 --
>  lib/eal/include/rte_debug.h             |  4 ----
>  lib/eal/ppc/include/rte_byteorder.h     |  9 ---------
>  lib/eal/x86/include/rte_byteorder.h     |  8 --------
>  lib/eal/x86/include/rte_vect.h          |  9 +--------
>  lib/log/rte_log.h                       |  4 ----
>  8 files changed, 1 insertion(+), 66 deletions(-)
> 
I think there are checks for GCC_VERSION > 40306 (i.e. gcc 4.3) in
rte_lru_x86.h that can be removed too. Otherwise,

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[PATCH v2] remove compatibility wrappers for GCC < 4.8

[David Marchand]

As we made our move to C11, GCC requirements has been bumped.
Wrappers for old versions of GCC can be dropped.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
Changes since v1:
- removed empty #else #endif block in x86 rte_byteorder.h,
- updated lib/table/rte_lru_x86.h (thanks Bruce),

---
 lib/eal/arm/include/rte_byteorder.h     | 19 -------------------
 lib/eal/arm/include/rte_vect.h          | 12 ------------
 lib/eal/include/generic/rte_byteorder.h |  2 --
 lib/eal/include/rte_debug.h             |  4 ----
 lib/eal/ppc/include/rte_byteorder.h     |  9 ---------
 lib/eal/x86/include/rte_byteorder.h     |  9 ---------
 lib/eal/x86/include/rte_vect.h          |  9 +--------
 lib/log/rte_log.h                       |  4 ----
 lib/table/rte_lru_x86.h                 |  4 ++--
 9 files changed, 3 insertions(+), 69 deletions(-)

diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h
index df2f1d87ba..ff02052f2e 100644
--- a/lib/eal/arm/include/rte_byteorder.h
+++ b/lib/eal/arm/include/rte_byteorder.h
@@ -17,25 +17,6 @@ extern "C" {
 #include <rte_common.h>
 #include "generic/rte_byteorder.h"
 
-/* fix missing __builtin_bswap16 for gcc older then 4.8 */
-#if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8))
-
-static inline uint16_t rte_arch_bswap16(uint16_t _x)
-{
-	uint16_t x = _x;
-
-	asm volatile ("rev16 %w0,%w1"
-		      : "=r" (x)
-		      : "r" (x)
-		      );
-	return x;
-}
-
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ? \
-				   rte_constant_bswap16(x) : \
-				   rte_arch_bswap16(x)))
-#endif
-
 /* ARM architecture is bi-endian (both big and little). */
 #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
 
diff --git a/lib/eal/arm/include/rte_vect.h b/lib/eal/arm/include/rte_vect.h
index 4b705bac5f..8cfe4bddc1 100644
--- a/lib/eal/arm/include/rte_vect.h
+++ b/lib/eal/arm/include/rte_vect.h
@@ -83,18 +83,6 @@ vcopyq_laneq_u32(uint32x4_t a, const int lane_a,
 #if defined(RTE_ARCH_ARM64)
 #if RTE_CC_IS_GNU && (GCC_VERSION < 70000)
 
-#if (GCC_VERSION < 40900)
-typedef uint64_t poly64_t;
-typedef uint64x2_t poly64x2_t;
-typedef uint8_t poly128_t __attribute__((vector_size(16), aligned(16)));
-
-static inline uint32x4_t
-vceqzq_u32(uint32x4_t a)
-{
-	return (a == 0);
-}
-#endif
-
 /* NEON intrinsic vreinterpretq_u64_p128() is supported since GCC version 7 */
 static inline uint64x2_t
 vreinterpretq_u64_p128(poly128_t x)
diff --git a/lib/eal/include/generic/rte_byteorder.h b/lib/eal/include/generic/rte_byteorder.h
index a67e1d70d9..03e9a24718 100644
--- a/lib/eal/include/generic/rte_byteorder.h
+++ b/lib/eal/include/generic/rte_byteorder.h
@@ -234,9 +234,7 @@ static uint64_t rte_be_to_cpu_64(rte_be64_t x);
 #endif /* __DOXYGEN__ */
 
 #ifdef RTE_FORCE_INTRINSICS
-#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)
 #define rte_bswap16(x) __builtin_bswap16(x)
-#endif
 
 #define rte_bswap32(x) __builtin_bswap32(x)
 
diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
index 2c4b94a7c9..74593cd4d4 100644
--- a/lib/eal/include/rte_debug.h
+++ b/lib/eal/include/rte_debug.h
@@ -60,11 +60,7 @@ void rte_dump_stack(void);
  * documentation.
  */
 void __rte_panic(const char *funcname , const char *format, ...)
-#ifdef __GNUC__
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
 	__rte_cold
-#endif
-#endif
 	__rte_noreturn
 	__rte_format_printf(2, 3);
 
diff --git a/lib/eal/ppc/include/rte_byteorder.h b/lib/eal/ppc/include/rte_byteorder.h
index 49f369f1e7..de94e2ad32 100644
--- a/lib/eal/ppc/include/rte_byteorder.h
+++ b/lib/eal/ppc/include/rte_byteorder.h
@@ -62,15 +62,6 @@ static inline uint64_t rte_arch_bswap64(uint64_t _x)
 #define rte_bswap64(x) ((uint64_t)(__builtin_constant_p(x) ?		\
 				   rte_constant_bswap64(x) :		\
 				   rte_arch_bswap64(x)))
-#else
-/*
- * __builtin_bswap16 is only available gcc 4.8 and upwards
- */
-#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
-				   rte_constant_bswap16(x) :		\
-				   rte_arch_bswap16(x)))
-#endif
 #endif
 
 /* Power 8 have both little endian and big endian mode
diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
index a2dfecc1f5..1bab5f15fe 100644
--- a/lib/eal/x86/include/rte_byteorder.h
+++ b/lib/eal/x86/include/rte_byteorder.h
@@ -59,15 +59,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x)
 #define rte_bswap64(x) ((uint64_t)(__builtin_constant_p(x) ?		\
 				   rte_constant_bswap64(x) :		\
 				   rte_arch_bswap64(x)))
-#else
-/*
- * __builtin_bswap16 is only available gcc 4.8 and upwards
- */
-#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
-#define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ?		\
-				   rte_constant_bswap16(x) :		\
-				   rte_arch_bswap16(x)))
-#endif
 #endif
 
 #define rte_cpu_to_le_16(x) (x)
diff --git a/lib/eal/x86/include/rte_vect.h b/lib/eal/x86/include/rte_vect.h
index 2e40b77da9..560f9e4db3 100644
--- a/lib/eal/x86/include/rte_vect.h
+++ b/lib/eal/x86/include/rte_vect.h
@@ -16,20 +16,13 @@
 #include <rte_common.h>
 #include "generic/rte_vect.h"
 
-#if (defined(__ICC) || \
-	(defined(_WIN64)) || \
-	(__GNUC__ == 4 &&  __GNUC_MINOR__ < 4))
-
+#if defined(__ICC) || defined(_WIN64)
 #include <smmintrin.h> /* SSE4 */
-
 #if defined(__AVX__)
 #include <immintrin.h>
 #endif
-
 #else
-
 #include <x86intrin.h>
-
 #endif
 
 #ifdef __cplusplus
diff --git a/lib/log/rte_log.h b/lib/log/rte_log.h
index bdefff2a59..f7a8405de9 100644
--- a/lib/log/rte_log.h
+++ b/lib/log/rte_log.h
@@ -277,11 +277,7 @@ void rte_log_dump(FILE *f);
  *   - Negative on error.
  */
 int rte_log(uint32_t level, uint32_t logtype, const char *format, ...)
-#ifdef __GNUC__
-#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
 	__rte_cold
-#endif
-#endif
 	__rte_format_printf(3, 4);
 
 /**
diff --git a/lib/table/rte_lru_x86.h b/lib/table/rte_lru_x86.h
index 38476d956e..ddfb8c1c8c 100644
--- a/lib/table/rte_lru_x86.h
+++ b/lib/table/rte_lru_x86.h
@@ -20,7 +20,7 @@ extern "C" {
 
 #if RTE_TABLE_HASH_LRU_STRATEGY == 2
 
-#if RTE_CC_IS_GNU && (GCC_VERSION > 40306)
+#if RTE_CC_IS_GNU
 #include <x86intrin.h>
 #else
 #include <emmintrin.h>
@@ -64,7 +64,7 @@ do {									\
 
 #elif RTE_TABLE_HASH_LRU_STRATEGY == 3
 
-#if RTE_CC_IS_GNU && (GCC_VERSION > 40306)
+#if RTE_CC_IS_GNU
 #include <x86intrin.h>
 #else
 #include <emmintrin.h>
-- 
2.41.0

Re: [PATCH v2] remove compatibility wrappers for GCC < 4.8

[David Marchand]

On Thu, Aug 24, 2023 at 11:34 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> As we made our move to C11, GCC requirements has been bumped.
> Wrappers for old versions of GCC can be dropped.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks.


-- 
David Marchand

Re: [PATCH] remove wrappers for GCC < 4.8

[Tyler Retzlaff]

On Thu, Aug 24, 2023 at 10:52:04AM +0200, David Marchand wrote:
> Hello Tyler,
> 
> On Thu, Aug 24, 2023 at 10:30 AM David Marchand
> <david.marchand@redhat.com> wrote:
> > diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
> > index 2c4b94a7c9..74593cd4d4 100644
> > --- a/lib/eal/include/rte_debug.h
> > +++ b/lib/eal/include/rte_debug.h
> > @@ -60,11 +60,7 @@ void rte_dump_stack(void);
> >   * documentation.
> >   */
> >  void __rte_panic(const char *funcname , const char *format, ...)
> > -#ifdef __GNUC__
> > -#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
> >         __rte_cold
> > -#endif
> > -#endif
> 
> I don't see some wrapping around __rte_cold for MSVC in your series.
> Would this patch break MSVC buidlds?

If it gets expanded on MSVC probably. But don't let it stop you from
applying this series. You can just expand it empty if you want.

Until the atomics series is merged msvc still won't build so I'll supply
minor fixes as these things pop up.

Once everything is staged I'll work with UNH to get a basic build CI
going to reduce the overhead of worrying about this kind of change being
breaking.

> 
> >         __rte_noreturn
> >         __rte_format_printf(2, 3);
> >
> 
> 
> -- 
> David Marchand

Re: [PATCH] remove wrappers for GCC < 4.8

[David Marchand]

On Fri, Aug 25, 2023 at 6:14 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
> On Thu, Aug 24, 2023 at 10:52:04AM +0200, David Marchand wrote:
> > Hello Tyler,
> >
> > On Thu, Aug 24, 2023 at 10:30 AM David Marchand
> > <david.marchand@redhat.com> wrote:
> > > diff --git a/lib/eal/include/rte_debug.h b/lib/eal/include/rte_debug.h
> > > index 2c4b94a7c9..74593cd4d4 100644
> > > --- a/lib/eal/include/rte_debug.h
> > > +++ b/lib/eal/include/rte_debug.h
> > > @@ -60,11 +60,7 @@ void rte_dump_stack(void);
> > >   * documentation.
> > >   */
> > >  void __rte_panic(const char *funcname , const char *format, ...)
> > > -#ifdef __GNUC__
> > > -#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ > 2))
> > >         __rte_cold
> > > -#endif
> > > -#endif
> >
> > I don't see some wrapping around __rte_cold for MSVC in your series.
> > Would this patch break MSVC buidlds?
>
> If it gets expanded on MSVC probably. But don't let it stop you from
> applying this series. You can just expand it empty if you want.

That's what I did for safety when I applied the msvc series.

>
> Until the atomics series is merged msvc still won't build so I'll supply
> minor fixes as these things pop up.
>
> Once everything is staged I'll work with UNH to get a basic build CI
> going to reduce the overhead of worrying about this kind of change being
> breaking.

Yes, the sooner, the better.


-- 
David Marchand