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 10:53 am, Jan Beulich wrote:
> On 11.09.2023 11:48, Andrew Cooper wrote:
>> 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.
> Hmm, even in forward order one prior #elif will need touching

 #if GCC_VERSION < 40700
 #define GCOV_COUNTERS 8
 #elif GCC_VERSION < 40900
 #define GCOV_COUNTERS 9
 #elif GCC_VERSION < 50000
 #define GCOV_COUNTERS 10
 #elif GCC_VERSION < 70000
 #define GCOV_COUNTERS 9
+#elif GCC_VERSION < FOO
+#define GCOV_COUNTERS BAR
 #else
 #error "Wrong version of GCC used to compile gcov"
 #endif

~Andrew