Re: [Minios-devel] [UNIKRAT RFC PATCH] build: Introduce flag CFLAGS_REMOVE to remove some options

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hi Justin,

thanks a lot for sharing this. I have the concern that introducing 
"remove flag" is making the build system a bit in-transparent and hard 
to debug and understandable for developers. I think for each flag there 
is a good reason why it got introduced. It doesn't feel right that other 
libraries are able to remove some of them again.
The original discussion was coming from the fact that we need to split 
our code in pieces that need to be ISR-safe (mainly using only small 
subset of CPU features/registers that is saved no interrupt requests) 
and other parts that can use extended CPU features (e.g., floating 
point, vector registers). For this purpose, I would prefer that we 
introduce a scheme where we can compile "ISR-safe" files or functions. 
The arch library would populate different optimization flags to those 
units, e.g., with a new ISR_CFLAGS-y variable.
I think it is quite easy to achieve this on a source file level, at 
least we should do this. Preferably we should achieve this on a function 
declaration level. Are there GCC flags/attributes we could use to mark 
and compile a function ISR-safe (while the rest of the file is using all 
optimization features)?
Thanks,

Simon

On 24.02.20 07:36, Justin He wrote:
> Hi Simon and Felipe
> This patch is the proposal as we discussed in last meeting.
> After this patch, in newlib Makefile, we need to add
> Ifeq (ARM_64, y)
> CFLAGS_REMOVE=-mgeneral-regs-only
> endif
>
> What do you think of it
> --
> Cheers,
> Justin (Jia He)
>
>
> > -----Original Message-----
> > From: Jia He <justin.he@xxxxxxx>
> > Sent: Monday, February 24, 2020 2:31 PM
> > To: minios-devel@xxxxxxxxxxxxxxxxxxxx; Simon Kuenzer
> > <simon.kuenzer@xxxxxxxxx>; Santiago.Pagani@xxxxxxxxx
> > Cc: Sharan.Santhanam@xxxxxxxxx; Felipe Huici <felipe.huici@xxxxxxxxx>;
> > Julien Grall <Julien.Grall@xxxxxxx>; Kaly Xin <Kaly.Xin@xxxxxxx>; Justin He
> > <Justin.He@xxxxxxx>
> > Subject: [UNIKRAT RFC PATCH] build: Introduce flag CFLAGS_REMOVE to
> > remove some options
> >
> > Previously, there is no way to remove any global options for individual
> > library. For some reason, on arm64 we need to use mgeneral-regs-only
> > for unikraft plat library (context switch, print debug). But for some
> > other libraries (e.g. newlib), we shouldn't use that option due to
> > float point/simd support on arm64.
> >
> > This patch adds a new flag CFLAGS_REMOVE to filter it out in unikraft
> > build system.
> >
> > Signed-off-by: Jia He <justin.he@xxxxxxx>
> > ---
> >   support/build/Makefile.rules | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/support/build/Makefile.rules b/support/build/Makefile.rules
> > index d60c6d9..dea2c7f 100644
> > --- a/support/build/Makefile.rules
> > +++ b/support/build/Makefile.rules
> > @@ -391,7 +391,9 @@ $(3): $(2) | prepare
> >         $(call build_cmd_fixdep,CC,$(1),$(3),\
> >                 $(CC)  $(CINCLUDES) $(CINCLUDES-y) \
> >                        $($(call vprefix_lib,$(1),CINCLUDES)) $($(call
> > vprefix_lib,$(1),CINCLUDES-y)) \
> > -                      $(CFLAGS) $(CFLAGS-y) $(DBGFLAGS) $(DBGFLAGS-y) \
> > +                      $(filter-out $(CFLAGS_REMOVE),$(CFLAGS)) \
> > +                       $(filter-out $(CFLAGS_REMOVE),$(CFLAGS-y)) \
> > +                       $(DBGFLAGS) $(DBGFLAGS-y) \
> >                        $($(call vprefix_lib,$(1),CFLAGS)) $($(call
> > vprefix_lib,$(1),CFLAGS-y)) \
> >                        $(4) -D__LIBNAME__=$(1) -D__BASENAME__=$(notdir $(2))
> > \
> >                        -c $(2) -o $(3) $(depflags)
> > --
> > 2.17.1
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel