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 Otherwise a nit and a question: Parentheses would want using consistently. And wouldn't it make sense to combine the two ranges resulting in 9 being chosen? (Imo in the alternative layout that's less desirable.) Since the adjustment would be easy to make, I'd be fine doing so while committing, and then Reviewed-by: Jan Beulich <[email protected]> As an unrelated remark: gcc_3_4.c is clearly outdated as well, simply by its name. Imo it would have wanted to be gcc_4_1.c the latest as of commit 03f22f0070f3 ("README: adjust gcc version requirement"), i.e. for over 10 years. Jan
