On 11.09.2023 11:59, Andrew Cooper wrote: > 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 <[email protected]> >>>>> --- >>>>> 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
But that doesn't achieve the same as the other construct: No #error for gcc earlier than 4.7, and your (presumed; i.e. taking just patch context without the two added lines) construct would fail to build for gcc 7. We want an open upper bound but a determined lower one. Jan
