Re: [PATCH v4 1/3] kbuild: Add __cc-option macro

Masahiro Yamada
2017-06-20T09:38:10Z

Hi Matthias,

2017-06-20 3:37 GMT+09:00 Matthias Kaehlcke <mka@chromium.org>:
> cc-option uses KBUILD_CFLAGS and KBUILD_CPPFLAGS when it determines
> whether an option is supported or not. This is fine for options used to
> build the kernel itself, however some components like the x86 boot code
> use a different set of flags.
>
> Add the new macro __cc-option which is a more generic version of
> cc-option with additional parameters. One parameter is the compiler
> with which the check should be performed, the other the compiler options
> to be used instead KBUILD_C*FLAGS.
>
> Refactor cc-option and hostcc-option to use __cc-option and move
> hostcc-option to scripts/Kbuild.include.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> Changes in v4:
> - Remove extra space before alternative option in cc-option and
> hostcc-option
>
> Makefile | 2 +-
> scripts/Kbuild.include | 14 ++++++++++++--
> scripts/Makefile.host | 6 ------
> 3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 83f6d9972cab..b234bba6d652 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -303,7 +303,7 @@ CONFIG_SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
>
> HOSTCC = gcc
> HOSTCXX = g++
> -HOSTCFLAGS = -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89
> +HOSTCFLAGS := -Wall -Wmissing-prototypes -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu89
> HOSTCXXFLAGS = -O2
>
> ifeq ($(shell $(HOSTCC) -v 2>&1 | grep -c "clang version"), 1)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 61f87a99bf0a..81a58d1f53af 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -108,6 +108,11 @@ as-option = $(call try-run,\
> as-instr = $(call try-run,\
> printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
>
> +# __cc-option
> +# Usage: MY_CFLAGS += $(call __cc-option,$(CC),$(MY_CFLAGS),-march=winchip-c6,-march=i586)
> +__cc-option = $(call try-run,\
> + $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
> +
> # Do not attempt to build with gcc plugins during cc-option tests.
> # (And this uses delayed resolution so the flags will be up to date.)
> CC_OPTION_CFLAGS = $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
> @@ -115,8 +120,13 @@ CC_OPTION_CFLAGS = $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
> # cc-option
> # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>
> -cc-option = $(call try-run,\
> - $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +cc-option = $(call __cc-option, $(CC), $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS),\
> + $(1),$(2))

I think this will introduce an extra tab for the true case.

Could you wrap the line after $(CC),?

cc-option = $(call __cc-option, $(CC),\
$(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS),$(1),$(2))

I would not have requested v5 only for this,
but it looks like you have a chance for re-spin to improve 3/3.

--
Best Regards
Masahiro Yamada