[PATCH v3 06/11] eal: typedef cpu flag enum as int for msvc

[Konstantin Ananyev]

10/04/2023 21:53, Tyler Retzlaff пишет:
> On Mon, Apr 10, 2023 at 08:59:33PM +0100, Konstantin Ananyev wrote:
>> 06/04/2023 01:45, Tyler Retzlaff пишет:
>>> Forward declaration of a typedef is a non-standard extension and is not
>>> supported by msvc. Use an int instead.
>>>
>>> Abstract the use of the int/enum rte_cpu_flag_t in function parameter
>>> lists by re-typdefing the enum rte_cpu_flag_t to the rte_cpu_flag_t
>>> identifier.
>>>
>>> Remove the use of __extension__ on function prototypes where
>>> rte_cpu_flag_t appeared in parameter lists, it is sufficient to have the
>>> conditionally compiled __extension__ at the non-standard forward
>>> declaration site.
>>>
>>> Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
>>> ---
>>>   lib/eal/include/generic/rte_cpuflags.h | 12 +++++++-----
>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/eal/include/generic/rte_cpuflags.h b/lib/eal/include/generic/rte_cpuflags.h
>>> index d35551e..87ab03c 100644
>>> --- a/lib/eal/include/generic/rte_cpuflags.h
>>> +++ b/lib/eal/include/generic/rte_cpuflags.h
>>> @@ -44,8 +44,12 @@ struct rte_cpu_intrinsics {
>>>   /**
>>>    * Enumeration of all CPU features supported
>>>    */
>>> +#ifndef RTE_TOOLCHAIN_MSVC
>>>   __extension__
>>> -enum rte_cpu_flag_t;
>>> +typedef enum rte_cpu_flag_t rte_cpu_flag_t;
>>> +#else
>>> +typedef int rte_cpu_flag_t;
>>> +#endif
>>
>>
>> Just curious what exactly MSVC doesn't support here?
>> Is that construction like:
>> enum rte_cpu_flag_t {....};
>> enum rte_cpu_flag_t;
>> ...
>> Or something else?
> 
> Forward declaration of an enum is non standard. It's probably only
> allowed by gcc as an extension because gcc will make some kind of
> implementation specific promise for it always to be `int` size by
> default (assuming no other -foptions).

I understood that part, what is not clear to me from where we are 
getting forward declarations?
As I remember the usual organization of arch specific rte_cpuflags.h:

/* type definition */
enum rte_cpu_flag_t {...};

/some other stuff */

#include "generic/rte_cpuflags.h"

Which contains 'enum rte_cpu_flag_t' type declaration.
But it doesn't look like a forward declaration to me.
Is there a place where we do include "generic/rte_cpuflags.h" directly
(not from arch specific header)?
If so. might be we should change it to include arch specific header instead?

> 
> If the enum was defined before reference it would probably be accepted
> by msvc since it could 'see' the definition and know the integer width
> in use.
> 
>>
>>
>>>   /**
>>>    * Get name of CPU flag
>>> @@ -56,9 +60,8 @@ struct rte_cpu_intrinsics {
>>>    *     flag name
>>>    *     NULL if flag ID is invalid
>>>    */
>>> -__extension__
>>>   const char *
>>> -rte_cpu_get_flag_name(enum rte_cpu_flag_t feature);
>>> +rte_cpu_get_flag_name(rte_cpu_flag_t feature);
>>>   /**
>>>    * Function for checking a CPU flag availability
>>> @@ -70,9 +73,8 @@ struct rte_cpu_intrinsics {
>>>    *     0 if flag is not available
>>>    *     -ENOENT if flag is invalid
>>>    */
>>> -__extension__
>>>   int
>>> -rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature);
>>> +rte_cpu_get_flag_enabled(rte_cpu_flag_t feature);
>>>   /**
>>>    * This function checks that the currently used CPU supports the CPU features