Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/09/2023 8:54 am, Jan Beulich wrote:
> On 08.09.2023 18:20, Javi Merino wrote:
>> The current structure of choosing the correct file based on the
>> compiler version makes us make 33 line files just to define a
>> constant.  The changes after gcc 4.7 are minimal, just the number of
>> counters.
>>
>> Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
>> remove a lot of the boilerplate and keep the logic of choosing the
>> GCOV_COUNTER in gcc_4_7.c.
>>
>> Signed-off-by: Javi Merino <javi.merino@xxxxxxxxx>
>> ---
>>  xen/common/coverage/Makefile  |  6 +-----
>>  xen/common/coverage/gcc_4_7.c | 17 +++++++++--------
>>  xen/common/coverage/gcc_4_9.c | 33 ---------------------------------
>>  xen/common/coverage/gcc_5.c   | 33 ---------------------------------
>>  xen/common/coverage/gcc_7.c   | 30 ------------------------------
>>  5 files changed, 10 insertions(+), 109 deletions(-)
>>  delete mode 100644 xen/common/coverage/gcc_4_9.c
>>  delete mode 100644 xen/common/coverage/gcc_5.c
>>  delete mode 100644 xen/common/coverage/gcc_7.c
>>
>> diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
>> index 63f98c71d6..d729afc9c7 100644
>> --- a/xen/common/coverage/Makefile
>> +++ b/xen/common/coverage/Makefile
>> @@ -1,11 +1,7 @@
>>  obj-y += coverage.o
>>  ifneq ($(CONFIG_CC_IS_CLANG),y)
>>  obj-y += gcov_base.o gcov.o
>> -obj-y += $(call cc-ifversion,-lt,0407, \
>> -            gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
>> -            gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>> -            gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>> -            gcc_5.o, gcc_7.o))))
>> +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
>>  else
>>  obj-y += llvm.o
>>  endif
>> diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
>> index 25b4a8bcdc..ddbc9f4bb0 100644
>> --- a/xen/common/coverage/gcc_4_7.c
>> +++ b/xen/common/coverage/gcc_4_7.c
>> @@ -18,15 +18,16 @@
>>  
>>  #include "gcov.h"
>>  
>> -/*
>> - * GCOV_COUNTERS will be defined if this file is included by other
>> - * source files.
>> - */
>> -#ifndef GCOV_COUNTERS
>> -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>> -#  error "Wrong version of GCC used to compile gcov"
>> -# endif
>> +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>>  #define GCOV_COUNTERS 8
>> +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 50000)
>> +#define GCOV_COUNTERS 9
>> +#elif GCC_VERSION >= 50000 && GCC_VERSION < 70000
>> +#define GCOV_COUNTERS 10
>> +#elif GCC_VERSION >= 70000
>> +#define GCOV_COUNTERS 9
>> +#else
>> +#error "Wrong version of GCC used to compile gcov"
>>  #endif
> How about inverse order:
>
> #if GCC_VERSION >= 70000
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 50000
> #define GCOV_COUNTERS 10
> #elif GCC_VERSION >= 40900
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 40700
> #define GCOV_COUNTERS 8
> #else
> #error "Wrong version of GCC used to compile gcov"
> #endif

Forward order is the one that results in a smaller diff when inserting
new entries.

More importantly, it's the more natural way to structure such a list.

~Andrew