[PATCH for-8.2 v2 5/7] target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro

Daniel Henrique Barboza posted 7 patches 2 years, 7 months ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Cleber Rosa <crosa@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>
[PATCH for-8.2 v2 5/7] target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro
Posted by Daniel Henrique Barboza 2 years, 7 months ago
The code inside riscv_cpu_add_user_properties() became quite repetitive
after recent changes. Add a macro to hide the repetition away.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c0826b449d..b61465c8c4 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1881,6 +1881,11 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v,
 }
 #endif
 
+#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
+    for (prop = _array; prop && prop->name; prop++) { \
+        qdev_property_add_static(_dev, prop); \
+    } \
+
 /*
  * Add CPU properties with user-facing flags.
  *
@@ -1924,17 +1929,9 @@ static void riscv_cpu_add_user_properties(Object *obj)
         qdev_property_add_static(dev, prop);
     }
 
-    for (prop = riscv_cpu_options; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
-
-    for (prop = riscv_cpu_vendor_exts; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
-
-    for (prop = riscv_cpu_experimental_exts; prop && prop->name; prop++) {
-        qdev_property_add_static(dev, prop);
-    }
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_options);
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_vendor_exts);
+    ADD_CPU_PROPERTIES_ARRAY(dev, riscv_cpu_experimental_exts);
 }
 
 static Property riscv_cpu_properties[] = {
-- 
2.41.0
Re: [PATCH for-8.2 v2 5/7] target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro
Posted by Richard Henderson 2 years, 7 months ago
On 7/12/23 21:57, Daniel Henrique Barboza wrote:
> +#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
> +    for (prop = _array; prop && prop->name; prop++) { \
> +        qdev_property_add_static(_dev, prop); \
> +    } \

do { } while(0)

Watch the \ on the last line of the macro.
Declare the iterator within the macro, rather than use one defined in the outer scope.
Why not use ARRAY_SIZE?


r~
Re: [PATCH for-8.2 v2 5/7] target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro
Posted by Daniel Henrique Barboza 2 years, 7 months ago

On 7/13/23 17:40, Richard Henderson wrote:
> On 7/12/23 21:57, Daniel Henrique Barboza wrote:
>> +#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
>> +    for (prop = _array; prop && prop->name; prop++) { \
>> +        qdev_property_add_static(_dev, prop); \
>> +    } \
> 
> do { } while(0)
> 
> Watch the \ on the last line of the macro.
> Declare the iterator within the macro, rather than use one defined in the outer scope.

Like this?

#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
     do { \
         Property *prop; \
         for (prop = _array; prop && prop->name; prop++) { \
             qdev_property_add_static(_dev, prop); \
         } \
     } while(0)

> Why not use ARRAY_SIZE?

Hm, the arrays are finishing with DEFINE_PROP_END_OF_LIST() (I copied the existing
array structure), which adds an empty element, so ARRAY_SIZE will get empty stuff
in the end.

Since these are new arrays I can get rid of the end_of_list blank element and use
ARRAY_SIZE().


Daniel
> 
> 
> r~

Re: [PATCH for-8.2 v2 5/7] target/riscv/cpu.c: add a ADD_CPU_PROPERTIES_ARRAY() macro
Posted by Richard Henderson 2 years, 7 months ago
On 7/13/23 22:27, Daniel Henrique Barboza wrote:
> 
> 
> On 7/13/23 17:40, Richard Henderson wrote:
>> On 7/12/23 21:57, Daniel Henrique Barboza wrote:
>>> +#define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
>>> +    for (prop = _array; prop && prop->name; prop++) { \
>>> +        qdev_property_add_static(_dev, prop); \
>>> +    } \
>>
>> do { } while(0)
>>
>> Watch the \ on the last line of the macro.
>> Declare the iterator within the macro, rather than use one defined in the outer scope.
> 
> Like this?
> 
> #define ADD_CPU_PROPERTIES_ARRAY(_dev, _array) \
>      do { \
>          Property *prop; \
>          for (prop = _array; prop && prop->name; prop++) { \
>              qdev_property_add_static(_dev, prop); \
>          } \
>      } while(0)

Yes, like that, thanks.


r~