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

[Masahiro Yamada]

Hi Matthias,

2017-06-20 3:37 GMT+09:00 Matthias Kaehlcke <mka@xxxxxxxxxxxx>:
> 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@xxxxxxxx>
> Suggested-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> Acked-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> ---
> 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