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
