Re: [Minios-devel] [UNIKRAFT PATCH 5/9] build: Add a makefile rule to check valid gcc version

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Simon,

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2018年4月5日 5:47
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Shijie Huang <Shijie.Huang@xxxxxxx>; Kaly Xin <Kaly.Xin@xxxxxxx>; nd
> <nd@xxxxxxx>
> Subject: Re: [UNIKRAFT PATCH 5/9] build: Add a makefile rule to check valid
> gcc version
> 
> On 15.03.2018 04:39, Wei Chen wrote:
> > Different GCC versions support different march, mcpu and mtune list.
> > In order to avoid users to select unsupported optimization processor
> > for the GCC they used. We add this check_valid_gcc_version rule to
> > check GCC version and provide the minimum version for user selected
> > processor.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   Makefile                     | 4 ++++
> >   support/build/Makefile.rules | 8 ++++++++
> >   2 files changed, 12 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 49e5070..997a359 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -426,6 +426,10 @@ LIFTOFF                := liftoff -e -s
> >   override ARFLAGS:= rcs
> >   CC_VERSION        := $(shell $(CC) --version | \
> >                sed -n -r 's/^.* ([0-9]*)\.([0-9]*)\.([0-9]*)[ ]*.*/\1.\2/p')
> > +# Retrieve GCC major and minor number from CC_VERSION. They would be used
> > +# to select correct optimization parameters for target CPUs.
> > +CC_VER_MAJOR   := $(word 1,$(subst ., ,$(CC_VERSION)))
> > +CC_VER_MINOR   := $(word 2,$(subst ., ,$(CC_VERSION)))
> >
> >   ASFLAGS           += -DCC_VERSION=$(CC_VERSION)
> >   CFLAGS            += -DCC_VERSION=$(CC_VERSION)
> > diff --git a/support/build/Makefile.rules b/support/build/Makefile.rules
> > index e4ac264..e51f902 100644
> > --- a/support/build/Makefile.rules
> > +++ b/support/build/Makefile.rules
> > @@ -29,6 +29,14 @@ lc = $(subst A,a,$(subst B,b,$(subst C,c,$(subst
> D,d,$(subst E,e,$(subst F,f,$(s
> >   # upper case
> >   uc = $(subst a,A,$(subst b,B,$(subst c,C,$(subst d,D,$(subst e,E,$(subst
> f,F,$(subst g,G,$(subst h,H,$(subst i,I,$(subst j,J,$(subst k,K,$(subst
> l,L,$(subst m,M,$(subst n,N,$(subst o,O,$(subst p,P,$(subst q,Q,$(subst
> r,R,$(subst s,S,$(subst t,T,$(subst u,U,$(subst v,V,$(subst w,W,$(subst
> x,X,$(subst y,Y,$(subst z,Z,$(1)))))))))))))))))))))))))))
> >
> > +# check whether the GCC meets the minimum version requirement
> > +# $(1): The minimum GCC major number
> > +# $(2): The minimum GCC minor number
> > +# $(3): The optimization target CPU
> > +define check_valid_gcc_version =
> > +$(if $(shell test $(CC_VER_MAJOR) -gt $(1) -o '(' $(CC_VER_MAJOR) -eq $(1)
> -a $(CC_VER_MINOR) -ge $(2) ')' && echo y),,\
> > +     $(error Optimization for $(3) requires GCC version >= $(1).$(2)))
> > +endef
> 
> Hum, I think this could be useful to a lot of other places in Unikraft.
> But for this we should do a split. One definition should just do the
> test so that the result could be used for if statements.
> I would call this one 'test_gcc_version'.
> 

That's a good idea! I would split it.

> Yours should use 'test_gcc_version', and use $(error ) if GCC is too
> old. I would also the not talk about optimization in the error message.
> You could treat $(3) as optional argument or even remove it.
> 
> How about something like this:
> 
> Require GCC version >= $(1).$(2) (found $(CC_VER_MAJOR).$(CC_VER_MINOR))
> 

Yes, it seems much better than it was. I would do it in v2

> >
> >
> ##############################################################################
> ##
> >   #
> >
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel