[XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk

Anthony PERARD posted 15 patches 1 year, 6 months ago
[XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk
Posted by Anthony PERARD 1 year, 6 months ago
In xen/, it isn't necessary to include Config.mk in every Makefile in
subdirectories as nearly all necessary variables should be calculated
in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
that is only available in Config.mk.

Extract $(cc-option,) from Config.mk so we can use it without
including Config.mk and thus without having to recalculate some CFLAGS
which would be ignored.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 Config.mk                  | 27 +--------------------------
 config/compiler-testing.mk | 25 +++++++++++++++++++++++++
 xen/Rules.mk               |  1 +
 3 files changed, 27 insertions(+), 26 deletions(-)
 create mode 100644 config/compiler-testing.mk

diff --git a/Config.mk b/Config.mk
index 27f48f654a..ebc8d0dfdd 100644
--- a/Config.mk
+++ b/Config.mk
@@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "
 or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
 
 -include $(XEN_ROOT)/.config
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
@@ -79,32 +80,6 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)"
 # to permit the user to set PYTHON_PREFIX_ARG to '' to workaround this bug:
 #  https://bugs.launchpad.net/ubuntu/+bug/362570
 
-# cc-option: Check if compiler supports first option, else fall back to second.
-#
-# This is complicated by the fact that unrecognised -Wno-* options:
-#   (a) are ignored unless the compilation emits a warning; and
-#   (b) even then produce a warning rather than an error
-# To handle this we do a test compile, passing the option-under-test, on a code
-# fragment that will always produce a warning (integer assigned to pointer).
-# We then grep for the option-under-test in the compiler's output, the presence
-# of which would indicate an "unrecognized command-line option" warning/error.
-#
-# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
-cc-option = $(shell if test -z "`echo 'void*p=1;' | \
-              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \
-              then echo "$(2)"; else echo "$(3)"; fi ;)
-
-# cc-option-add: Add an option to compilation flags, but only if supported.
-# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
-cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
-define cc-option-add-closure
-    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
-        $(1) += $(3)
-    endif
-endef
-
-cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
-
 # cc-ver: Check compiler against the version requirement. Return boolean 'y'/'n'.
 # Usage: ifeq ($(call cc-ver,$(CC),ge,0x030400),y)
 cc-ver = $(shell if [ $$((`$(1) -dumpversion | awk -F. \
diff --git a/config/compiler-testing.mk b/config/compiler-testing.mk
new file mode 100644
index 0000000000..9677f91abe
--- /dev/null
+++ b/config/compiler-testing.mk
@@ -0,0 +1,25 @@
+# cc-option: Check if compiler supports first option, else fall back to second.
+#
+# This is complicated by the fact that unrecognised -Wno-* options:
+#   (a) are ignored unless the compilation emits a warning; and
+#   (b) even then produce a warning rather than an error
+# To handle this we do a test compile, passing the option-under-test, on a code
+# fragment that will always produce a warning (integer assigned to pointer).
+# We then grep for the option-under-test in the compiler's output, the presence
+# of which would indicate an "unrecognized command-line option" warning/error.
+#
+# Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
+cc-option = $(shell if test -z "`echo 'void*p=1;' | \
+              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \
+              then echo "$(2)"; else echo "$(3)"; fi ;)
+
+# cc-option-add: Add an option to compilation flags, but only if supported.
+# Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
+cc-option-add = $(eval $(call cc-option-add-closure,$(1),$(2),$(3)))
+define cc-option-add-closure
+    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
+        $(1) += $(3)
+    endif
+endef
+
+cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 68b10ca5ef..8177d405c3 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -19,6 +19,7 @@ __build:
 
 include $(XEN_ROOT)/Config.mk
 include $(srctree)/scripts/Kbuild.include
+include $(XEN_ROOT)/config/compiler-testing.mk
 
 # Initialise some variables
 obj-y :=
-- 
Anthony PERARD
Re: [XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk
Posted by Jan Beulich 1 year, 6 months ago
On 23.05.2023 18:38, Anthony PERARD wrote:
> In xen/, it isn't necessary to include Config.mk in every Makefile in
> subdirectories as nearly all necessary variables should be calculated
> in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
> that is only available in Config.mk.
> 
> Extract $(cc-option,) from Config.mk so we can use it without
> including Config.mk and thus without having to recalculate some CFLAGS
> which would be ignored.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

When I saw the title and in particular the new file's name which is
already mentioned there, I did expect something entirely different,
perhaps related to the testing of Xen. May I suggest e.g.
compiler-probing.mk or maybe even simply compiler.mk? (I'm also
uncertain about "compiler", tbh - I could e.g. see Kbuild.include's
as-option-add to also be useful outside of xen/, and hence at some
point wanting moving to some common file.)

> --- a/Config.mk
> +++ b/Config.mk
> @@ -18,6 +18,7 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir $(file)) && echo "
>  or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(3),$(if $(strip $(4)),$(4)))))
>  
>  -include $(XEN_ROOT)/.config
> +include $(XEN_ROOT)/config/compiler-testing.mk

Possibly being one of the few users of the top-level .config, I wonder
about the ordering of these includes. This isn't to say that I consider
wrong the order in which you have it now, but I could see the opposite
order as potentially useful, too.

Jan
Re: [XEN PATCH 14/15] Config.mk: move $(cc-option, ) to config/compiler-testing.mk
Posted by Luca Fancellu 1 year, 6 months ago

> On 23 May 2023, at 17:38, Anthony PERARD <anthony.perard@citrix.com> wrote:
> 
> In xen/, it isn't necessary to include Config.mk in every Makefile in
> subdirectories as nearly all necessary variables should be calculated
> in xen/Makefile. But some Makefile make use of the macro $(cc-option,)
> that is only available in Config.mk.
> 
> Extract $(cc-option,) from Config.mk so we can use it without
> including Config.mk and thus without having to recalculate some CFLAGS
> which would be ignored.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>