Re: [XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
On 15.04.2020 16:10, Anthony PERARD wrote: > On Wed, Apr 08, 2020 at 01:50:33PM +0200, Jan Beulich wrote: >> On 31.03.2020 12:30, Anthony PERARD wrote: >>> # Always build obj-bin files as binary even if they come from C source. >>> -$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) >>> +# FIXME LTO broken, but we would need a different way to filter -flto out >>> +# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) >> >> While you mention this in the description, I'm still not overly >> happy with it getting commented out. What's wrong with making the >> construct simply act on c_flags? > > It doesn't work. > > I tried > $(obj-bin-y): c_flags := $(filter-out -flto,$(c_flags)) > but the $@ expansion was empty. Hmm, yes, presumably because of having to use :=. But the old code gives the appearance of having worked despite this fact. > I guess we could do: > $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) > that's probably enough for now. Even if we can't test it properly. If -flto gets added to XEN_CFLAGS (not c_flags) - why not? Jan
Re: [XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
On Wed, Apr 08, 2020 at 01:50:33PM +0200, Jan Beulich wrote: > On 31.03.2020 12:30, Anthony PERARD wrote: > > # Always build obj-bin files as binary even if they come from C source. > > -$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) > > +# FIXME LTO broken, but we would need a different way to filter -flto out > > +# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) > > While you mention this in the description, I'm still not overly > happy with it getting commented out. What's wrong with making the > construct simply act on c_flags? It doesn't work. I tried $(obj-bin-y): c_flags := $(filter-out -flto,$(c_flags)) but the $@ expansion was empty. I guess we could do: $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS)) that's probably enough for now. Even if we can't test it properly. -- Anthony PERARD
Re: [XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
On 31.03.2020 12:30, Anthony PERARD wrote: > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -115,6 +115,64 @@ $(KCONFIG_CONFIG): > include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG) > $(MAKE) $(kconfig) syncconfig > > +ifeq ($(CONFIG_DEBUG),y) > +CFLAGS += -O1 > +else > +CFLAGS += -O2 > +endif > + > +ifeq ($(CONFIG_FRAME_POINTER),y) > +CFLAGS += -fno-omit-frame-pointer > +else > +CFLAGS += -fomit-frame-pointer > +endif > + > +CFLAGS += -nostdinc -fno-builtin -fno-common > +CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith > +$(call cc-option-add,CFLAGS,CC,-Wvla) > +CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h > +CFLAGS-$(CONFIG_DEBUG_INFO) += -g > + > +ifneq ($(CONFIG_CC_IS_CLANG),y) > +# Clang doesn't understand this command line argument, and doesn't appear to > +# have an suitable alternative. The resulting compiled binary does function, I realize you only move this, but it would still be nice to adjust this to "... a suitable alternative" on this occasion. > +# but has an excessively large symbol table. > +CFLAGS += -Wa,--strip-local-absolute > +endif > + > +AFLAGS += -D__ASSEMBLY__ > + > +CFLAGS += $(CFLAGS-y) > +# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE > +CFLAGS += $(EXTRA_CFLAGS_XEN_CORE) > + > +# Most CFLAGS are safe for assembly files: > +# -std=gnu{89,99} gets confused by #-prefixed end-of-line comments > +# -flto makes no sense and annoys clang > +AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y) > + > +# LDFLAGS are only passed directly to $(LD) > +LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y) > + > +ifeq ($(CONFIG_UBSAN),y) > +CFLAGS_UBSAN := -fsanitize=undefined > +else > +CFLAGS_UBSAN := > +endif > + > +ifeq ($(CONFIG_LTO),y) > +CFLAGS += -flto > +LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so > +endif > + > +include $(BASEDIR)/arch/$(TARGET_ARCH)/arch.mk > + > +# define new variables to avoid the ones defines in Config.mk s/defines/defined/ > @@ -140,10 +95,19 @@ obj-bin-y := > endif > > # Always build obj-bin files as binary even if they come from C source. > -$(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) > +# FIXME LTO broken, but we would need a different way to filter -flto out > +# $(obj-bin-y): CFLAGS := $(filter-out -flto,$(CFLAGS)) While you mention this in the description, I'm still not overly happy with it getting commented out. What's wrong with making the construct simply act on c_flags? Jan
[XEN PATCH v4 06/18] xen/build: have the root Makefile generates the CFLAGS
Instead of generating the CFLAGS in Rules.mk everytime we enter a new subdirectory, we are going to generate most of them a single time, and export the result in the environment so that Rules.mk can use it. The only flags left to be generated are the ones that depend on the targets, but the variable $(c_flags) takes care of that. Arch specific CFLAGS are generated by a new file "arch/*/arch.mk" which is included by the root Makefile. We export the *FLAGS via the environment variables XEN_*FLAGS because Rules.mk still includes Config.mk and would add duplicated flags to CFLAGS. When running Rules.mk in the root directory (xen/), the variable `root-make-done' is set, so `need-config' will remain undef and so the root Makefile will not generate the cflags again. We can't use CFLAGS in subdirectories to add flags to particular targets, instead start to use CFLAGS-y. Idem for AFLAGS. So there are two different CFLAGS-y, the one in xen/Makefile (and arch.mk), and the one in subdirs that Rules.mk is going to use. We can't add to XEN_CFLAGS because it is exported, so making change to it might be propagated to subdirectory which isn't intended. Some style change are introduced in this patch: when LDFLAGS_DIRECT is included in LDFLAGS use of CFLAGS-$(CONFIG_INDIRECT_THUNK) instead of ifeq(). There is on FIXME added about LTO build, but since LTO is marked as BROKEN, this commit doesn't attempt to filter -flto flags out of the CFLAGS. Signed-off-by: Anthony PERARD --- Notes: v4: - typos - Adding $(AFLAGS-y) to $(AFLAGS) v3: - squash "xen/build: introduce ccflags-y and CFLAGS_$@" here, with those changes: - rename ccflags-y to simply CFLAGS-y and start using AFLAGS-y in subdirs. - remove CFLAGS_$@, we don't need it yet. - fix build of xen.lds and efi.lds which needed -D to be a_flags - remove arch_ccflags, and modify c_flags directly with that change, reorder c_flags, so that target specific flags are last. - remove HAVE_AS_QUOTED_SYM from envvar and check XEN_CFLAGS to find if it's there when adding -D__OBJECT_LABEL__. - fix missing some flags in AFLAGS (like -fshort-wchar in xen/arch/x86/efi/Makefile, and -D__OBJECT_LABEL__ and CFLAGS-stack-boundary) - keep COV_FLAGS generation in Rules.mk since it doesn't invovle to call CC - fix clang test for "asm()-s support .include." (in a new patch done ahead) - include Kconfig.include in xen/Makefile because as-option-add is defined there now. xen/Makefile | 58 +++ xen/Rules.mk | 74 +++- xen/arch/arm/Makefile | 10 ++-- xen/arch/arm/Rules.mk | 23 xen/arch/arm/{Rules.mk => arch.mk} | 5 -- xen/arch/arm/efi/Makefile | 2 +- xen/arch/x86/Makefile | 24 xen/arch/x86/Rules.mk | 91 ++ xen/arch/x86/{Rules.mk => arch.mk} | 17 ++ xen/arch/x86/efi/Makefile | 2 +- xen/common/libelf/Makefile | 4 +- xen/common/libfdt/Makefile | 4 +- xen/include/Makefile | 2 +- xen/xsm/flask/Makefile | 2 +- xen/xsm/flask/ss/Makefile | 2 +- 15 files changed, 115 insertions(+), 205 deletions(-) copy xen/arch/arm/{Rules.mk => arch.mk} (85%) copy xen/arch/x86/{Rules.mk => arch.mk} (87%) diff --git a/xen/Makefile b/xen/Makefile index 8375070e0d41..372692841913 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -115,6 +115,64 @@ $(KCONFIG_CONFIG): include/config/%.conf include/config/%.conf.cmd: $(KCONFIG_CONFIG) $(MAKE) $(kconfig) syncconfig +ifeq ($(CONFIG_DEBUG),y) +CFLAGS += -O1 +else +CFLAGS += -O2 +endif + +ifeq ($(CONFIG_FRAME_POINTER),y) +CFLAGS += -fno-omit-frame-pointer +else +CFLAGS += -fomit-frame-pointer +endif + +CFLAGS += -nostdinc -fno-builtin -fno-common +CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith +$(call cc-option-add,CFLAGS,CC,-Wvla) +CFLAGS += -pipe -D__XEN__ -include $(BASEDIR)/include/xen/config.h +CFLAGS-$(CONFIG_DEBUG_INFO) += -g + +ifneq ($(CONFIG_CC_IS_CLANG),y) +# Clang doesn't understand this command line argument, and doesn't appear to +# have an suitable alternative. The resulting compiled binary does function, +# but has an excessively large symbol table. +CFLAGS += -Wa,--strip-local-absolute +endif + +AFLAGS += -D__ASSEMBLY__ + +CFLAGS += $(CFLAGS-y) +# allow extra CFLAGS externally via EXTRA_CFLAGS_XEN_CORE +CFLAGS += $(EXTRA_CFLAGS_XEN_CORE) + +# Most CFLAGS are safe for assembly files: +# -std=gnu{89,99} gets confused by #-prefixed end-of-line comments +# -flto makes no sense and annoys clang +AFLAGS += $(filter-out -std=gnu% -flto,$(CFLAGS)) $(AFLAGS-y) + +# LDFLAGS are only passed directly to $(LD) +LDFLAGS += $(LDFLAGS_DIRECT) $(LDFLAGS-y) + +ifeq ($(CONFIG_UBSAN),y) +CFLAGS_UBSAN := -fsanitize=undefined +else +CFLAGS_UBSAN := +endif