| Message ID | 20210824105038.1257926-2-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen: Build system improvements, now with out-of-tree build! | expand |
On 24.08.2021 12:49, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> albeit with a remark: > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -133,6 +133,9 @@ endif > # Always build obj-bin files as binary even if they come from C source. > $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) > > +# To be use with $(a_flags) or $(c_flags) to produce CPP flags > +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1)) Afaics this has nothing to do with Linux'es cpp_flags, so what we do here is entirely up to us. If this is strictly intended to be used the another macro, wouldn't it make sense to have cpp_flags = $(filter-out -Wa$(comma)% -flto,$($(1))) here and then e.g. ... > @@ -222,13 +225,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE > $(call if_changed,obj_init_o) > > quiet_cmd_cpp_i_c = CPP $@ > -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -MQ $@ -o $@ $< > +cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $< ... the slightly simpler / easier to read cmd_cpp_i_c = $(CPP) $(call cpp_flags,c_flags) -MQ $@ -o $@ $< here? Jan
On Thu, Sep 02, 2021 at 12:08:58PM +0200, Jan Beulich wrote: > On 24.08.2021 12:49, Anthony PERARD wrote: > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > albeit with a remark: > > > --- a/xen/Rules.mk > > +++ b/xen/Rules.mk > > @@ -133,6 +133,9 @@ endif > > # Always build obj-bin files as binary even if they come from C source. > > $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) > > > > +# To be use with $(a_flags) or $(c_flags) to produce CPP flags > > +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1)) > > Afaics this has nothing to do with Linux'es cpp_flags, so what we do here > is entirely up to us. If this is strictly intended to be used the another > macro, wouldn't it make sense to have > > cpp_flags = $(filter-out -Wa$(comma)% -flto,$($(1))) > > here and then e.g. ... > > > @@ -222,13 +225,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE > > $(call if_changed,obj_init_o) > > > > quiet_cmd_cpp_i_c = CPP $@ > > -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -MQ $@ -o $@ $< > > +cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $< > > ... the slightly simpler / easier to read > > cmd_cpp_i_c = $(CPP) $(call cpp_flags,c_flags) -MQ $@ -o $@ $< > > here? I don't think this is better or simpler. "cpp_flags" don't need to know the name of the variable to be useful. I think it is better to know that "cpp_flags" act on the value of the variable rather than the variable itself, when reading "$(call cpp_flags, $(a_flags))". But thanks for the suggestion, and for the review,
On 06.09.2021 12:06, Anthony PERARD wrote: > On Thu, Sep 02, 2021 at 12:08:58PM +0200, Jan Beulich wrote: >> On 24.08.2021 12:49, Anthony PERARD wrote: >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> >> Reviewed-by: Jan Beulich <jbeulich@suse.com> >> albeit with a remark: >> >>> --- a/xen/Rules.mk >>> +++ b/xen/Rules.mk >>> @@ -133,6 +133,9 @@ endif >>> # Always build obj-bin files as binary even if they come from C source. >>> $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) >>> >>> +# To be use with $(a_flags) or $(c_flags) to produce CPP flags >>> +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1)) >> >> Afaics this has nothing to do with Linux'es cpp_flags, so what we do here >> is entirely up to us. If this is strictly intended to be used the another >> macro, wouldn't it make sense to have >> >> cpp_flags = $(filter-out -Wa$(comma)% -flto,$($(1))) >> >> here and then e.g. ... >> >>> @@ -222,13 +225,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE >>> $(call if_changed,obj_init_o) >>> >>> quiet_cmd_cpp_i_c = CPP $@ >>> -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -MQ $@ -o $@ $< >>> +cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $< >> >> ... the slightly simpler / easier to read >> >> cmd_cpp_i_c = $(CPP) $(call cpp_flags,c_flags) -MQ $@ -o $@ $< >> >> here? > > I don't think this is better or simpler. "cpp_flags" don't need to know > the name of the variable to be useful. I think it is better to know that > "cpp_flags" act on the value of the variable rather than the variable > itself, when reading "$(call cpp_flags, $(a_flags))". Well, yes. This way one could also pass more than just the expansion of either of these two variables. The thing that made me think of the alternative is the comment: Would you mind if I inserted "e.g." in there, to make clear this isn't limited to these two variables? Jan
On Mon, Sep 06, 2021 at 12:30:07PM +0200, Jan Beulich wrote: > On 06.09.2021 12:06, Anthony PERARD wrote: > > On Thu, Sep 02, 2021 at 12:08:58PM +0200, Jan Beulich wrote: > >> On 24.08.2021 12:49, Anthony PERARD wrote: > >>> +# To be use with $(a_flags) or $(c_flags) to produce CPP flags > >>> +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1)) [..] > Well, yes. This way one could also pass more than just the expansion of > either of these two variables. The thing that made me think of the > alternative is the comment: Would you mind if I inserted "e.g." in there, > to make clear this isn't limited to these two variables? Adding "e.g." is fine. Thanks.
diff --git a/xen/Makefile b/xen/Makefile index 94e837182615..4ceb02d37441 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -410,7 +410,7 @@ include/xen/compile.h: include/xen/compile.h.in .banner @mv -f $@.new $@ asm-offsets.s: arch/$(TARGET_ARCH)/$(TARGET_SUBARCH)/asm-offsets.c - $(CC) $(filter-out -Wa$(comma)% -flto,$(c_flags)) -S -g0 -o $@.new -MQ $@ $< + $(CC) $(call cpp_flags,$(c_flags)) -S -g0 -o $@.new -MQ $@ $< $(call move-if-changed,$@.new,$@) include/asm-$(TARGET_ARCH)/asm-offsets.h: asm-offsets.s diff --git a/xen/Rules.mk b/xen/Rules.mk index d65d6a48993b..c99c4a9475c9 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -133,6 +133,9 @@ endif # Always build obj-bin files as binary even if they come from C source. $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) +# To be use with $(a_flags) or $(c_flags) to produce CPP flags +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1)) + # Calculation of flags, first the generic flags, then the arch specific flags, # and last the flags modified for a target or a directory. @@ -222,13 +225,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o FORCE $(call if_changed,obj_init_o) quiet_cmd_cpp_i_c = CPP $@ -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -MQ $@ -o $@ $< +cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $< quiet_cmd_cc_s_c = CC $@ cmd_cc_s_c = $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -S $< -o $@ quiet_cmd_cpp_s_S = CPP $@ -cmd_cpp_s_S = $(CPP) $(filter-out -Wa$(comma)%,$(a_flags)) -MQ $@ -o $@ $< +cmd_cpp_s_S = $(CPP) $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $< %.i: %.c FORCE $(call if_changed,cpp_i_c) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index fe38cfd54421..462472215c91 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -292,7 +292,7 @@ $(BASEDIR)/include/asm-x86/asm-macros.h: asm-macros.i Makefile efi.lds: AFLAGS-y += -DEFI xen.lds efi.lds: xen.lds.S - $(CPP) -P $(filter-out -Wa$(comma)%,$(a_flags)) -MQ $@ -o $@ $< + $(CPP) -P $(call cpp_flags,$(a_flags)) -MQ $@ -o $@ $< boot/mkelf32: boot/mkelf32.c $(HOSTCC) $(HOSTCFLAGS) -o $@ $< diff --git a/xen/arch/x86/mm/Makefile b/xen/arch/x86/mm/Makefile index b31041644fe8..2818c066f76a 100644 --- a/xen/arch/x86/mm/Makefile +++ b/xen/arch/x86/mm/Makefile @@ -15,7 +15,7 @@ guest_walk_%.o: guest_walk.c Makefile $(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_walk_%.i: guest_walk.c Makefile - $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ + $(CPP) $(call cpp_flags,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_walk_%.s: guest_walk.c Makefile $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@ diff --git a/xen/arch/x86/mm/hap/Makefile b/xen/arch/x86/mm/hap/Makefile index 22e7ad54bd33..c6d296b51720 100644 --- a/xen/arch/x86/mm/hap/Makefile +++ b/xen/arch/x86/mm/hap/Makefile @@ -9,7 +9,7 @@ guest_walk_%level.o: guest_walk.c Makefile $(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_walk_%level.i: guest_walk.c Makefile - $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ + $(CPP) $(call cpp_flags,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_walk_%level.s: guest_walk.c Makefile $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@ diff --git a/xen/arch/x86/mm/shadow/Makefile b/xen/arch/x86/mm/shadow/Makefile index 770213fe9d84..fd64b4dda925 100644 --- a/xen/arch/x86/mm/shadow/Makefile +++ b/xen/arch/x86/mm/shadow/Makefile @@ -10,7 +10,7 @@ guest_%.o: multi.c Makefile $(CC) $(c_flags) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_%.i: multi.c Makefile - $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ + $(CPP) $(call cpp_flags,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -c $< -o $@ guest_%.s: multi.c Makefile $(CC) $(filter-out -Wa$(comma)%,$(c_flags)) -DGUEST_PAGING_LEVELS=$* -S $< -o $@
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v7: - also filter -flto - and convert "asm-offsets.s" and "xen/x86/%.lds" to use cpp_flags as well. v6: - switch to a macro as suggested which allows to be used with both a_flags and c_flags v5: - new patch xen/Makefile | 2 +- xen/Rules.mk | 7 +++++-- xen/arch/x86/Makefile | 2 +- xen/arch/x86/mm/Makefile | 2 +- xen/arch/x86/mm/hap/Makefile | 2 +- xen/arch/x86/mm/shadow/Makefile | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-)