Re: [PATCH v2 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros

2022-09-05 Thread Masahiro Yamada
On Thu, Sep 1, 2022 at 3:44 AM Nick Desaulniers  wrote:
>
> cc-ifversion is GCC specific. Replace it with compiler specific
> variants. Update the users of cc-ifversion to use these new macros.
> Provide a helper for checking compiler versions for GCC and Clang
> simultaneously, that will be used in a follow up patch.
>
> Cc: Jonathan Corbet 
> Cc: linux-...@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Link: https://github.com/ClangBuiltLinux/linux/issues/350
> Link: 
> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=b...@mail.gmail.com/
> Suggested-by: Bill Wendling 
> Signed-off-by: Nick Desaulniers 
> ---
> Changes v1 -> v2:
> * New patch.
>
>  Documentation/kbuild/makefiles.rst  | 44 +++--
>  Makefile|  4 +-
>  drivers/gpu/drm/amd/display/dc/dml/Makefile | 12 ++
>  scripts/Makefile.compiler   | 15 +--
>  4 files changed, 49 insertions(+), 26 deletions(-)
>
> diff --git a/Documentation/kbuild/makefiles.rst 
> b/Documentation/kbuild/makefiles.rst
> index 11a296e52d68..e46f5b45c422 100644
> --- a/Documentation/kbuild/makefiles.rst
> +++ b/Documentation/kbuild/makefiles.rst
> @@ -682,22 +682,42 @@ more details, with real examples.
> In the above example, -Wno-unused-but-set-variable will be added to
> KBUILD_CFLAGS only if gcc really accepts it.
>
> -cc-ifversion
> -   cc-ifversion tests the version of $(CC) and equals the fourth 
> parameter
> -   if version expression is true, or the fifth (if given) if the version
> -   expression is false.
> +gcc-min-version
> +   gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is 
> greater than
> +   or equal to the provided value and evaluates to y if so.
>
> Example::
>
> -   #fs/reiserfs/Makefile
> -   ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
> +   cflags-$(call gcc-min-version, 70100) := -foo
>
> -   In this example, ccflags-y will be assigned the value -O1 if the
> -   $(CC) version is less than 4.2.
> -   cc-ifversion takes all the shell operators:
> -   -eq, -ne, -lt, -le, -gt, and -ge
> -   The third parameter may be a text as in this example, but it may also
> -   be an expanded variable or a macro.
> +   In this example, cflags-y will be assigned the value -foo if $(CC) is 
> gcc and
> +   $(CONFIG_GCC_VERSION) is >= 7.1.
> +
> +clang-min-version
> +   clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is 
> greater
> +   than or equal to the provided value and evaluates to y if so.
> +
> +   Example::
> +
> +   cflags-$(call clang-min-version, 11) := -foo
> +
> +   In this example, cflags-y will be assigned the value -foo if $(CC) is 
> clang
> +   and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
> +
> +cc-min-version
> +   cc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater
> +   than or equal to the first value provided, or if the value of
> +   $(CONFIG_CLANG_VERSION) is greater than or equal to the second value
> +   provided, and evaluates
> +   to y if so.
> +
> +   Example::
> +
> +   cflags-$(call cc-min-version, 70100, 11) := -foo
> +
> +   In this example, cflags-y will be assigned the value -foo if $(CC) is 
> gcc and
> +   $(CONFIG_GCC_VERSION) is >= 7.1, or if $(CC) is clang and
> +   $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>
>  cc-cross-prefix
> cc-cross-prefix is used to check if there exists a $(CC) in path with
> diff --git a/Makefile b/Makefile
> index 952d354069a4..caa39ecb1136 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -972,7 +972,7 @@ ifdef CONFIG_CC_IS_GCC
>  KBUILD_CFLAGS += -Wno-maybe-uninitialized
>  endif
>
> -ifdef CONFIG_CC_IS_GCC
> +ifeq ($(call gcc-min-version, 90100),y)
>  # The allocators already balk at large sizes, so silence the compiler
>  # warnings for bounds checks involving those possible values. While
>  # -Wno-alloc-size-larger-than would normally be used here, earlier versions
> @@ -984,7 +984,7 @@ ifdef CONFIG_CC_IS_GCC
>  # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>  # choice, we must perform a versioned check to disable this warning.
>  # https://lore.kernel.org/lkml/20210824115859.187f2...@canb.auug.org.au
> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
> +KBUILD_CFLAGS += -Wno-alloc-size-larger-than
>  endif
>
>  # disable invalid "can't wrap" optimizations for signed / pointers
> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile 
> b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> index 86a3b5bfd699..d8ee4743b2e3 100644
> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
> @@ -33,20 +33,14 @@ ifdef CONFIG_PPC64
>  dml_ccflags := -mhard-float -maltivec
>  endif
>
> 

Re: [PATCH v2 3/5] Makefile.compiler: replace cc-ifversion with compiler-specific macros

2022-08-31 Thread Nathan Chancellor
On Wed, Aug 31, 2022 at 11:44:06AM -0700, Nick Desaulniers wrote:
> cc-ifversion is GCC specific. Replace it with compiler specific
> variants. Update the users of cc-ifversion to use these new macros.
> Provide a helper for checking compiler versions for GCC and Clang
> simultaneously, that will be used in a follow up patch.
> 
> Cc: Jonathan Corbet 
> Cc: linux-...@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: dri-de...@lists.freedesktop.org
> Link: https://github.com/ClangBuiltLinux/linux/issues/350
> Link: 
> https://lore.kernel.org/llvm/CAGG=3QWSAUakO42kubrCap8fp-gm1ERJJAYXTnP1iHk_wrH=b...@mail.gmail.com/
> Suggested-by: Bill Wendling 
> Signed-off-by: Nick Desaulniers 

These are so much nicer. I find the name kind of awkward but the only
thing I could come up with that sounded better was 'gcc-is-at-least' or
'clang-is-at-least' and I don't really feel like painting sheds today,
it's hot outside :)

Reviewed-by: Nathan Chancellor 

Some comments below.

> ---
> Changes v1 -> v2:
> * New patch.
> 
>  Documentation/kbuild/makefiles.rst  | 44 +++--
>  Makefile|  4 +-
>  drivers/gpu/drm/amd/display/dc/dml/Makefile | 12 ++
>  scripts/Makefile.compiler   | 15 +--
>  4 files changed, 49 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/kbuild/makefiles.rst 
> b/Documentation/kbuild/makefiles.rst
> index 11a296e52d68..e46f5b45c422 100644
> --- a/Documentation/kbuild/makefiles.rst
> +++ b/Documentation/kbuild/makefiles.rst
> @@ -682,22 +682,42 @@ more details, with real examples.
>   In the above example, -Wno-unused-but-set-variable will be added to
>   KBUILD_CFLAGS only if gcc really accepts it.
>  
> -cc-ifversion
> - cc-ifversion tests the version of $(CC) and equals the fourth parameter
> - if version expression is true, or the fifth (if given) if the version
> - expression is false.
> +gcc-min-version
> + gcc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater 
> than
> + or equal to the provided value and evaluates to y if so.
>  
>   Example::
>  
> - #fs/reiserfs/Makefile
> - ccflags-y := $(call cc-ifversion, -lt, 0402, -O1)
> + cflags-$(call gcc-min-version, 70100) := -foo
>  
> - In this example, ccflags-y will be assigned the value -O1 if the
> - $(CC) version is less than 4.2.
> - cc-ifversion takes all the shell operators:
> - -eq, -ne, -lt, -le, -gt, and -ge
> - The third parameter may be a text as in this example, but it may also
> - be an expanded variable or a macro.
> + In this example, cflags-y will be assigned the value -foo if $(CC) is 
> gcc and
> + $(CONFIG_GCC_VERSION) is >= 7.1.
> +
> +clang-min-version
> + clang-min-version tests if the value of $(CONFIG_CLANG_VERSION) is 
> greater
> + than or equal to the provided value and evaluates to y if so.
> +
> + Example::
> +
> + cflags-$(call clang-min-version, 11) := -foo
> +
> + In this example, cflags-y will be assigned the value -foo if $(CC) is 
> clang
> + and $(CONFIG_CLANG_VERSION) is >= 11.0.0.
> +
> +cc-min-version
> + cc-min-version tests if the value of $(CONFIG_GCC_VERSION) is greater
> + than or equal to the first value provided, or if the value of
> + $(CONFIG_CLANG_VERSION) is greater than or equal to the second value
> + provided, and evaluates
> + to y if so.
> +
> + Example::
> +
> + cflags-$(call cc-min-version, 70100, 11) := -foo
> +
> + In this example, cflags-y will be assigned the value -foo if $(CC) is 
> gcc and
> + $(CONFIG_GCC_VERSION) is >= 7.1, or if $(CC) is clang and
> + $(CONFIG_CLANG_VERSION) is >= 11.0.0.
>  
>  cc-cross-prefix
>   cc-cross-prefix is used to check if there exists a $(CC) in path with
> diff --git a/Makefile b/Makefile
> index 952d354069a4..caa39ecb1136 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -972,7 +972,7 @@ ifdef CONFIG_CC_IS_GCC
>  KBUILD_CFLAGS += -Wno-maybe-uninitialized
>  endif
>  
> -ifdef CONFIG_CC_IS_GCC
> +ifeq ($(call gcc-min-version, 90100),y)
>  # The allocators already balk at large sizes, so silence the compiler
>  # warnings for bounds checks involving those possible values. While
>  # -Wno-alloc-size-larger-than would normally be used here, earlier versions
> @@ -984,7 +984,7 @@ ifdef CONFIG_CC_IS_GCC
>  # ignored, continuing to default to PTRDIFF_MAX. So, left with no other
>  # choice, we must perform a versioned check to disable this warning.
>  # https://lore.kernel.org/lkml/20210824115859.187f2...@canb.auug.org.au
> -KBUILD_CFLAGS += $(call cc-ifversion, -ge, 0901, -Wno-alloc-size-larger-than)
> +KBUILD_CFLAGS += -Wno-alloc-size-larger-than
>  endif

It might be interesting to move this up to where KBUILD_CFLAGS-y is used
to make it:

  KBUILD_CFLAGS-$(call gcc-min-version, 90100) += -Wno-alloc-size-larger-than

But the comment