llvm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
@ 2023-06-08 17:01 Nathan Chancellor
  2023-06-08 17:21 ` Hamza Mahfooz
  2023-06-10  7:14 ` Jani Nikula
  0 siblings, 2 replies; 4+ messages in thread
From: Nathan Chancellor @ 2023-06-08 17:01 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, Xinhui.Pan
  Cc: ndesaulniers, trix, hamza.mahfooz, Kenny.Ho, amd-gfx, dri-devel,
	llvm, patches, Nathan Chancellor

-Wunused-but-set-variable was only supported in clang starting with
13.0.0, so earlier versions will emit a warning, which is turned into a
hard error for the kernel to mirror GCC:

  error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]

The minimum supported version of clang for building the kernel is
11.0.0, so match the rest of the kernel and wrap
-Wunused-but-set-variable in a cc-option call, so that it is only used
when supported by the compiler.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1869
Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 7ee68b1bbfed..86b833085f19 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
 	-I$(FULL_AMD_PATH)/amdkfd
 
 subdir-ccflags-y := -Wextra
-subdir-ccflags-y += -Wunused-but-set-variable
+subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
 subdir-ccflags-y += -Wno-unused-parameter
 subdir-ccflags-y += -Wno-type-limits
 subdir-ccflags-y += -Wno-sign-compare

---
base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363
change-id: 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
  2023-06-08 17:01 [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option Nathan Chancellor
@ 2023-06-08 17:21 ` Hamza Mahfooz
  2023-06-10  7:14 ` Jani Nikula
  1 sibling, 0 replies; 4+ messages in thread
From: Hamza Mahfooz @ 2023-06-08 17:21 UTC (permalink / raw)
  To: Nathan Chancellor, alexander.deucher, christian.koenig, Xinhui.Pan
  Cc: ndesaulniers, trix, Kenny.Ho, amd-gfx, dri-devel, llvm, patches

On 6/8/23 13:01, Nathan Chancellor wrote:
> -Wunused-but-set-variable was only supported in clang starting with
> 13.0.0, so earlier versions will emit a warning, which is turned into a
> hard error for the kernel to mirror GCC:
> 
>    error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
> 
> The minimum supported version of clang for building the kernel is
> 11.0.0, so match the rest of the kernel and wrap
> -Wunused-but-set-variable in a cc-option call, so that it is only used
> when supported by the compiler.
> 
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1869
> Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Applied, thanks!

> ---
>   drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 7ee68b1bbfed..86b833085f19 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
>   	-I$(FULL_AMD_PATH)/amdkfd
>   
>   subdir-ccflags-y := -Wextra
> -subdir-ccflags-y += -Wunused-but-set-variable
> +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
>   subdir-ccflags-y += -Wno-unused-parameter
>   subdir-ccflags-y += -Wno-type-limits
>   subdir-ccflags-y += -Wno-sign-compare
> 
> ---
> base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363
> change-id: 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8
> 
> Best regards,
-- 
Hamza


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
  2023-06-08 17:01 [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option Nathan Chancellor
  2023-06-08 17:21 ` Hamza Mahfooz
@ 2023-06-10  7:14 ` Jani Nikula
  2023-06-12 15:39   ` Nathan Chancellor
  1 sibling, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2023-06-10  7:14 UTC (permalink / raw)
  To: Nathan Chancellor, alexander.deucher, christian.koenig, Xinhui.Pan
  Cc: amd-gfx, trix, llvm, ndesaulniers, patches, dri-devel,
	Nathan Chancellor, hamza.mahfooz, Kenny.Ho

On Thu, 08 Jun 2023, Nathan Chancellor <nathan@kernel.org> wrote:
> -Wunused-but-set-variable was only supported in clang starting with
> 13.0.0, so earlier versions will emit a warning, which is turned into a
> hard error for the kernel to mirror GCC:
>
>   error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
>
> The minimum supported version of clang for building the kernel is
> 11.0.0, so match the rest of the kernel and wrap
> -Wunused-but-set-variable in a cc-option call, so that it is only used
> when supported by the compiler.

I wonder if there's a table somewhere listing all the warning options,
which GCC and Clang versions support them, and which versions have them
in -Wall and -Wextra. Would be really useful.

If there isn't one, it would be really helpful. *wink*.

BR,
Jani.


>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1869
> Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 7ee68b1bbfed..86b833085f19 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
>  	-I$(FULL_AMD_PATH)/amdkfd
>  
>  subdir-ccflags-y := -Wextra
> -subdir-ccflags-y += -Wunused-but-set-variable
> +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
>  subdir-ccflags-y += -Wno-unused-parameter
>  subdir-ccflags-y += -Wno-type-limits
>  subdir-ccflags-y += -Wno-sign-compare
>
> ---
> base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363
> change-id: 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8
>
> Best regards,

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option
  2023-06-10  7:14 ` Jani Nikula
@ 2023-06-12 15:39   ` Nathan Chancellor
  0 siblings, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2023-06-12 15:39 UTC (permalink / raw)
  To: Jani Nikula
  Cc: alexander.deucher, christian.koenig, Xinhui.Pan, amd-gfx, trix,
	llvm, ndesaulniers, patches, dri-devel, hamza.mahfooz, Kenny.Ho

On Sat, Jun 10, 2023 at 10:14:05AM +0300, Jani Nikula wrote:
> On Thu, 08 Jun 2023, Nathan Chancellor <nathan@kernel.org> wrote:
> > -Wunused-but-set-variable was only supported in clang starting with
> > 13.0.0, so earlier versions will emit a warning, which is turned into a
> > hard error for the kernel to mirror GCC:
> >
> >   error: unknown warning option '-Wunused-but-set-variable'; did you mean '-Wunused-const-variable'? [-Werror,-Wunknown-warning-option]
> >
> > The minimum supported version of clang for building the kernel is
> > 11.0.0, so match the rest of the kernel and wrap
> > -Wunused-but-set-variable in a cc-option call, so that it is only used
> > when supported by the compiler.
> 
> I wonder if there's a table somewhere listing all the warning options,
> which GCC and Clang versions support them, and which versions have them
> in -Wall and -Wextra. Would be really useful.

I don't think there is anything other than the official documentations for each listing
all the warning options. I know each version has its own documentation
for comparing warnings between releases but that is obviously tedious.

The clang -Wall question is easy enough to answer based on the test
case:

https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0/clang/test/Misc/warning-wall.c
https://github.com/llvm/llvm-project/blob/llvmorg-15.0.0/clang/test/Misc/warning-wall.c
https://github.com/llvm/llvm-project/blob/llvmorg-14.0.0/clang/test/Misc/warning-wall.c
https://github.com/llvm/llvm-project/blob/llvmorg-13.0.0/clang/test/Misc/warning-wall.c
https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/test/Misc/warning-wall.c
https://github.com/llvm/llvm-project/blob/llvmorg-11.0.0/clang/test/Misc/warning-wall.c

Clang has a tool, diagtool, that can print information about -Wextra,
but I do not ship it with the kernel.org LLVM releases, nor does Debian
it seems. On a recent clang-17 (the colors don't matter for this
exercise):

$ diagtool tree -Wextra

GREEN = enabled by default
YELLOW = disabled by default
RED = unimplemented (accepted for GCC compatibility)

-Wextra
  -Wdeprecated-copy
    -Wdeprecated-copy-with-user-provided-copy
  -Wmissing-field-initializers
  -Wignored-qualifiers
    -Wignored-reference-qualifiers
  -Winitializer-overrides
  -Wsemicolon-before-method-body
  -Wmissing-method-return-type
  -Wsign-compare
  -Wunused-parameter
  -Wunused-but-set-parameter
  -Wnull-pointer-arithmetic
    -Wgnu-null-pointer-arithmetic
  -Wnull-pointer-subtraction
  -Wempty-init-stmt
  -Wstring-concatenation
  -Wfuse-ld-path

Maybe some of that can be useful for future travelers.

> If there isn't one, it would be really helpful. *wink*.

Heh, that does sound like an interesting project but I am not sure I
have the bandwidth at the moment to do something like that, especially
since the number of warnings that are different between GCC and clang
are continuing to dwindle :)

Cheers,
Nathan

> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1869
> > Fixes: a0fd5a5f676c ("drm/amd/amdgpu: introduce DRM_AMDGPU_WERROR")
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> > index 7ee68b1bbfed..86b833085f19 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> > +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> > @@ -40,7 +40,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
> >  	-I$(FULL_AMD_PATH)/amdkfd
> >  
> >  subdir-ccflags-y := -Wextra
> > -subdir-ccflags-y += -Wunused-but-set-variable
> > +subdir-ccflags-y += $(call cc-option, -Wunused-but-set-variable)
> >  subdir-ccflags-y += -Wno-unused-parameter
> >  subdir-ccflags-y += -Wno-type-limits
> >  subdir-ccflags-y += -Wno-sign-compare
> >
> > ---
> > base-commit: 6bd4b01e8938779b0d959bdf33949a9aa258a363
> > change-id: 20230608-amdgpu-wrap-wunused-but-set-variable-in-cc-option-0be9528ac5c8
> >
> > Best regards,
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-06-12 15:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 17:01 [PATCH] drm/amdgpu: Wrap -Wunused-but-set-variable in cc-option Nathan Chancellor
2023-06-08 17:21 ` Hamza Mahfooz
2023-06-10  7:14 ` Jani Nikula
2023-06-12 15:39   ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).