Re: [PATCH] eal/arm: replace RTE_BUILD_BUG on non-constant

[Stephen Hemminger]

On Thu, 2 May 2024 18:44:20 +0100
Daniel Gregory <daniel.greg...@bytedance.com> wrote:

> On Thu, May 02, 2024 at 09:20:45AM -0700, Stephen Hemminger wrote:
> > Why not:
> > diff --git a/lib/eal/arm/include/rte_pause_64.h 
> > b/lib/eal/arm/include/rte_pause_64.h
> > index 5cb8b59056..81987de771 100644
> > --- a/lib/eal/arm/include/rte_pause_64.h
> > +++ b/lib/eal/arm/include/rte_pause_64.h
> > @@ -172,6 +172,8 @@ rte_wait_until_equal_32(volatile uint32_t *addr, 
> > uint32_t expected,
> >  {
> >         uint32_t value;
> >  
> > +       static_assert(__builtin_constant_p(memorder), "memory order is not 
> > a constant");
> > +
> >         RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire &&
> >                 memorder != rte_memory_order_relaxed);
> >  
> > @@ -191,6 +193,8 @@ rte_wait_until_equal_64(volatile uint64_t *addr, 
> > uint64_t expected,
> >  {
> >         uint64_t value;
> >  
> > +       static_assert(__builtin_constant_p(memorder), "memory order is not 
> > a constant");
> > +
> >         RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire &&
> >                 memorder != rte_memory_order_relaxed);
> >    
> 
> What toolchain are you using? With your change I still get errors about
> the expression not being constant:
> 
> In file included from ../lib/eal/arm/include/rte_pause.h:13,
>                  from ../lib/eal/include/generic/rte_spinlock.h:25,
>                  from ../lib/eal/arm/include/rte_spinlock.h:17,
>                  from ../lib/telemetry/telemetry.c:20:
> ../lib/eal/arm/include/rte_pause_64.h: In function ‘rte_wait_until_equal_16’:
> ../lib/eal/arm/include/rte_pause_64.h:156:23: error: expression in static 
> assertion is not constant
> 156 |           static_assert(__builtin_constant_p(memorder), "memory order 
> is not a constant");
>     |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> I'm cross-compiling with GCC v12.2 using the
> config/arm/arm64_armv8_linux_gcc cross-file, and enabling
> RTE_ARM_USE_WFE by uncommenting it in config/arm/meson.build and setting
> its value to true.

I don't do ARM any more, suppose could build on Raspberry Pi but havent.

There are already constant checks like this elsewhere in the file. Why not this:
diff --git a/lib/eal/arm/include/rte_pause_64.h 
b/lib/eal/arm/include/rte_pause_64.h
index 5cb8b59056..4f54f5dac3 100644
--- a/lib/eal/arm/include/rte_pause_64.h
+++ b/lib/eal/arm/include/rte_pause_64.h
@@ -153,6 +153,7 @@ rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t 
expected,
 {
        uint16_t value;
 
+       RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder));
        RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire &&
                memorder != rte_memory_order_relaxed);
 
@@ -172,6 +173,7 @@ rte_wait_until_equal_32(volatile uint32_t *addr, uint32_t 
expected,
 {
        uint32_t value;
 
+       RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder));
        RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire &&
                memorder != rte_memory_order_relaxed);
 
@@ -191,6 +193,7 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t 
expected,
 {
        uint64_t value;
 
+       RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder));
        RTE_BUILD_BUG_ON(memorder != rte_memory_order_acquire &&
                memorder != rte_memory_order_relaxed);