LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] drm/i915: Clean up disabled warnings
@ 2021-08-24 23:22 Nathan Chancellor
  2021-08-25 23:03 ` Nick Desaulniers
  0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2021-08-24 23:22 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
  Cc: Nick Desaulniers, intel-gfx, dri-devel, linux-kernel,
	clang-built-linux, llvm, Nathan Chancellor

i915 enables a wider set of warnings with '-Wall -Wextra' then disables
several with cc-disable-warning. If an unknown flag gets added to
KBUILD_CFLAGS when building with clang, all subsequent calls to
cc-{disable-warning,option} will fail, meaning that all of these
warnings do not get disabled [1].

A separate series will address the root cause of the issue by not adding
these flags when building with clang [2]; however, the symptom of these
extra warnings appearing can be addressed separately by just removing
the calls to cc-disable-warning, which makes the build ever so slightly
faster because the compiler does not need to be called as much before
building.

The following warnings are supported by GCC 4.9 and clang 10.0.1, which
are the minimum supported versions of these compilers so the call to
cc-disable-warning is not necessary. Masahiro cleaned this up for the
reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
warning flags unconditionally").

* -Wmissing-field-initializers
* -Wsign-compare
* -Wtype-limits
* -Wunused-parameter

-Wunused-but-set-variable was implemented in clang 13.0.0 and
-Wframe-address was implemented in clang 12.0.0 so the
cc-disable-warning calls are kept for these two warnings.

Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
which is disabled for the specific files that are problematic. clang
added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
can be removed.

[1]: https://lore.kernel.org/r/202108210311.CBtcgoUL-lkp@intel.com/
[2]: https://lore.kernel.org/r/20210824022640.2170859-1-nathan@kernel.org/

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---

NOTE: This is based on my series to enable -Wsometimes-initialized here:

https://lore.kernel.org/r/20210824225427.2065517-1-nathan@kernel.org/

I sent it separately as this can go into whatever release but I would
like for that series to go into 5.15.

 drivers/gpu/drm/i915/Makefile | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 335ba9f43d8f..6b38547543b1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -13,13 +13,11 @@
 # will most likely get a sudden build breakage... Hopefully we will fix
 # new warnings before CI updates!
 subdir-ccflags-y := -Wall -Wextra
-subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
-subdir-ccflags-y += $(call cc-disable-warning, type-limits)
-subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
+subdir-ccflags-y += -Wno-unused-parameter
+subdir-ccflags-y += -Wno-type-limits
+subdir-ccflags-y += -Wno-missing-field-initializers
+subdir-ccflags-y += -Wno-sign-compare
 subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
-# clang warnings
-subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
-subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
 subdir-ccflags-y += $(call cc-disable-warning, frame-address)
 subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
 

base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
prerequisite-patch-id: 31c28450ed7e8785dce967a16db6d52eff3d7d6d
prerequisite-patch-id: 372dfa0e07249f207acc1942ab0e39b13ff229b2
prerequisite-patch-id: 1a585fa6cda50c32ad1e3ac8235d3cff1b599978
-- 
2.33.0


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

* Re: [PATCH] drm/i915: Clean up disabled warnings
  2021-08-24 23:22 [PATCH] drm/i915: Clean up disabled warnings Nathan Chancellor
@ 2021-08-25 23:03 ` Nick Desaulniers
  2021-08-25 23:07   ` Nathan Chancellor
  0 siblings, 1 reply; 3+ messages in thread
From: Nick Desaulniers @ 2021-08-25 23:03 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx, dri-devel,
	linux-kernel, clang-built-linux, llvm

On Tue, Aug 24, 2021 at 4:23 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> i915 enables a wider set of warnings with '-Wall -Wextra' then disables
> several with cc-disable-warning. If an unknown flag gets added to
> KBUILD_CFLAGS when building with clang, all subsequent calls to
> cc-{disable-warning,option} will fail, meaning that all of these
> warnings do not get disabled [1].
>
> A separate series will address the root cause of the issue by not adding
> these flags when building with clang [2]; however, the symptom of these
> extra warnings appearing can be addressed separately by just removing
> the calls to cc-disable-warning, which makes the build ever so slightly
> faster because the compiler does not need to be called as much before
> building.
>
> The following warnings are supported by GCC 4.9 and clang 10.0.1, which
> are the minimum supported versions of these compilers so the call to
> cc-disable-warning is not necessary. Masahiro cleaned this up for the
> reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
> warning flags unconditionally").
>
> * -Wmissing-field-initializers
> * -Wsign-compare
> * -Wtype-limits
> * -Wunused-parameter
>
> -Wunused-but-set-variable was implemented in clang 13.0.0 and
> -Wframe-address was implemented in clang 12.0.0 so the
> cc-disable-warning calls are kept for these two warnings.
>
> Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
> which is disabled for the specific files that are problematic. clang
> added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
> can be removed.
>
> [1]: https://lore.kernel.org/r/202108210311.CBtcgoUL-lkp@intel.com/
> [2]: https://lore.kernel.org/r/20210824022640.2170859-1-nathan@kernel.org/
>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch! Do you need to re-ping, rebase, or resend that
other series?
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> NOTE: This is based on my series to enable -Wsometimes-initialized here:
>
> https://lore.kernel.org/r/20210824225427.2065517-1-nathan@kernel.org/
>
> I sent it separately as this can go into whatever release but I would
> like for that series to go into 5.15.
>
>  drivers/gpu/drm/i915/Makefile | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 335ba9f43d8f..6b38547543b1 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -13,13 +13,11 @@
>  # will most likely get a sudden build breakage... Hopefully we will fix
>  # new warnings before CI updates!
>  subdir-ccflags-y := -Wall -Wextra
> -subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
> -subdir-ccflags-y += $(call cc-disable-warning, type-limits)
> -subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> +subdir-ccflags-y += -Wno-unused-parameter
> +subdir-ccflags-y += -Wno-type-limits
> +subdir-ccflags-y += -Wno-missing-field-initializers
> +subdir-ccflags-y += -Wno-sign-compare
>  subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
> -# clang warnings
> -subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
> -subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
>  subdir-ccflags-y += $(call cc-disable-warning, frame-address)
>  subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>
>
> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
> prerequisite-patch-id: 31c28450ed7e8785dce967a16db6d52eff3d7d6d
> prerequisite-patch-id: 372dfa0e07249f207acc1942ab0e39b13ff229b2
> prerequisite-patch-id: 1a585fa6cda50c32ad1e3ac8235d3cff1b599978
> --
> 2.33.0
>


-- 
Thanks,
~Nick Desaulniers

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

* Re: [PATCH] drm/i915: Clean up disabled warnings
  2021-08-25 23:03 ` Nick Desaulniers
@ 2021-08-25 23:07   ` Nathan Chancellor
  0 siblings, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2021-08-25 23:07 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx, dri-devel,
	linux-kernel, clang-built-linux, llvm

On 8/25/2021 4:03 PM, Nick Desaulniers wrote:
> On Tue, Aug 24, 2021 at 4:23 PM Nathan Chancellor <nathan@kernel.org> wrote:
>>
>> i915 enables a wider set of warnings with '-Wall -Wextra' then disables
>> several with cc-disable-warning. If an unknown flag gets added to
>> KBUILD_CFLAGS when building with clang, all subsequent calls to
>> cc-{disable-warning,option} will fail, meaning that all of these
>> warnings do not get disabled [1].
>>
>> A separate series will address the root cause of the issue by not adding
>> these flags when building with clang [2]; however, the symptom of these
>> extra warnings appearing can be addressed separately by just removing
>> the calls to cc-disable-warning, which makes the build ever so slightly
>> faster because the compiler does not need to be called as much before
>> building.
>>
>> The following warnings are supported by GCC 4.9 and clang 10.0.1, which
>> are the minimum supported versions of these compilers so the call to
>> cc-disable-warning is not necessary. Masahiro cleaned this up for the
>> reset of the kernel in commit 4c8dd95a723d ("kbuild: add some extra
>> warning flags unconditionally").
>>
>> * -Wmissing-field-initializers
>> * -Wsign-compare
>> * -Wtype-limits
>> * -Wunused-parameter
>>
>> -Wunused-but-set-variable was implemented in clang 13.0.0 and
>> -Wframe-address was implemented in clang 12.0.0 so the
>> cc-disable-warning calls are kept for these two warnings.
>>
>> Lastly, -Winitializer-overrides is clang's version of -Woverride-init,
>> which is disabled for the specific files that are problematic. clang
>> added a compatibility alias in clang 8.0.0 so -Winitializer-overrides
>> can be removed.
>>
>> [1]: https://lore.kernel.org/r/202108210311.CBtcgoUL-lkp@intel.com/
>> [2]: https://lore.kernel.org/r/20210824022640.2170859-1-nathan@kernel.org/
>>
>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Thanks for the patch! Do you need to re-ping, rebase, or resend that
> other series?
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

I assume you mean the series below rather than above? I sent this patch 
right after that series and it has one set of reviews so I am hoping the 
i915 maintainers will pick them up soon so this one can be applied 
afterwards or resent.

Thank you for the review!

Cheers,
Nathan

>> ---
>>
>> NOTE: This is based on my series to enable -Wsometimes-initialized here:
>>
>> https://lore.kernel.org/r/20210824225427.2065517-1-nathan@kernel.org/
>>
>> I sent it separately as this can go into whatever release but I would
>> like for that series to go into 5.15.
>>
>>   drivers/gpu/drm/i915/Makefile | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index 335ba9f43d8f..6b38547543b1 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -13,13 +13,11 @@
>>   # will most likely get a sudden build breakage... Hopefully we will fix
>>   # new warnings before CI updates!
>>   subdir-ccflags-y := -Wall -Wextra
>> -subdir-ccflags-y += $(call cc-disable-warning, unused-parameter)
>> -subdir-ccflags-y += $(call cc-disable-warning, type-limits)
>> -subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
>> +subdir-ccflags-y += -Wno-unused-parameter
>> +subdir-ccflags-y += -Wno-type-limits
>> +subdir-ccflags-y += -Wno-missing-field-initializers
>> +subdir-ccflags-y += -Wno-sign-compare
>>   subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
>> -# clang warnings
>> -subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
>> -subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
>>   subdir-ccflags-y += $(call cc-disable-warning, frame-address)
>>   subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>>
>>
>> base-commit: fb43ebc83e069625cfeeb2490efc3ffa0013bfa4
>> prerequisite-patch-id: 31c28450ed7e8785dce967a16db6d52eff3d7d6d
>> prerequisite-patch-id: 372dfa0e07249f207acc1942ab0e39b13ff229b2
>> prerequisite-patch-id: 1a585fa6cda50c32ad1e3ac8235d3cff1b599978
>> --
>> 2.33.0
>>
> 
> 

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

end of thread, other threads:[~2021-08-25 23:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 23:22 [PATCH] drm/i915: Clean up disabled warnings Nathan Chancellor
2021-08-25 23:03 ` Nick Desaulniers
2021-08-25 23:07   ` 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).