Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full") enabled extra warnings for i915 to spot possible
bugs in new code, and then disabled a subset of these warnings to keep
the current code building without warnings (with gcc). Enabling the
extra warnings also enabled some additional clang-only warnings, as a
result building i915 with clang currently is extremely noisy. For now
also disable the clang warnings sign-compare, sometimes-uninitialized,
unneeded-internal-declaration and initializer-overrides. If desired
they can be re-enabled after the code has been fixed.
Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
warnings to full")
Signed-off-by: Matthias Kaehlcke <[email protected]>
---
Changes in v2:
- rebased on drm-tip
- added comment indicating that disabled warnings are clang warnings
drivers/gpu/drm/i915/Makefile | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 4eee91a3a236..9717c037b582 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -18,6 +18,11 @@ subdir-ccflags-y += $(call cc-disable-warning, type-limits)
subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
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, sometimes-uninitialized)
+subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
+subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
# Fine grained warnings disable
--
2.16.2.804.g6dcf76e118-goog
On Mon, Mar 19, 2018 at 12:14:51PM -0700, Matthias Kaehlcke wrote:
> Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> warnings to full") enabled extra warnings for i915 to spot possible
> bugs in new code, and then disabled a subset of these warnings to keep
> the current code building without warnings (with gcc). Enabling the
> extra warnings also enabled some additional clang-only warnings, as a
> result building i915 with clang currently is extremely noisy. For now
> also disable the clang warnings sign-compare, sometimes-uninitialized,
> unneeded-internal-declaration and initializer-overrides. If desired
> they can be re-enabled after the code has been fixed.
>
> Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> warnings to full")
> Signed-off-by: Matthias Kaehlcke <[email protected]>
> ---
> Changes in v2:
> - rebased on drm-tip
> - added comment indicating that disabled warnings are clang warnings
>
> drivers/gpu/drm/i915/Makefile | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 4eee91a3a236..9717c037b582 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -18,6 +18,11 @@ subdir-ccflags-y += $(call cc-disable-warning, type-limits)
> subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
> 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, sometimes-uninitialized)
> +subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
> +subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
> subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
>
> # Fine grained warnings disable
Ping, it seems this one swept under the radar
Thanks
Matthias
Quoting Matthias Kaehlcke (2018-04-30 20:31:19)
> On Mon, Mar 19, 2018 at 12:14:51PM -0700, Matthias Kaehlcke wrote:
> > Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> > warnings to full") enabled extra warnings for i915 to spot possible
> > bugs in new code, and then disabled a subset of these warnings to keep
> > the current code building without warnings (with gcc). Enabling the
> > extra warnings also enabled some additional clang-only warnings, as a
> > result building i915 with clang currently is extremely noisy. For now
> > also disable the clang warnings sign-compare, sometimes-uninitialized,
> > unneeded-internal-declaration and initializer-overrides. If desired
> > they can be re-enabled after the code has been fixed.
> >
> > Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> > warnings to full")
Do we need to backport this for a non-default build with a non-default
compiler?
> > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > ---
> > Changes in v2:
> > - rebased on drm-tip
> > - added comment indicating that disabled warnings are clang warnings
> >
> > drivers/gpu/drm/i915/Makefile | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 4eee91a3a236..9717c037b582 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -18,6 +18,11 @@ subdir-ccflags-y += $(call cc-disable-warning, type-limits)
> > subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> > subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
> > subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
> > +# clang warnings
> > +subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
Too much mixup in the code to be fixed overnight indeed.
> > +subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
Annoyingly it appears that clang has more false positives.
> > +subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
Example? I don't recall this one, so don't know if we should just not
fix it rather than suppress. I've used ignored-attributes, perhaps that
was for the same cause.
> > +subdir-ccflags-y += $(call cc-disable-warning, initializer-overrides)
While we used per-file to restrict this one, I don't think we care that
much for precision with clang as well.
-Chris
On Mon, Apr 30, 2018 at 09:01:49PM +0100, Chris Wilson wrote:
> Quoting Matthias Kaehlcke (2018-04-30 20:31:19)
> > On Mon, Mar 19, 2018 at 12:14:51PM -0700, Matthias Kaehlcke wrote:
> > > Commit 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> > > warnings to full") enabled extra warnings for i915 to spot possible
> > > bugs in new code, and then disabled a subset of these warnings to keep
> > > the current code building without warnings (with gcc). Enabling the
> > > extra warnings also enabled some additional clang-only warnings, as a
> > > result building i915 with clang currently is extremely noisy. For now
> > > also disable the clang warnings sign-compare, sometimes-uninitialized,
> > > unneeded-internal-declaration and initializer-overrides. If desired
> > > they can be re-enabled after the code has been fixed.
> > >
> > > Fixes: 39bf4de89ff7 ("drm/i915: Add -Wall -Wextra to our build, set
> > > warnings to full")
>
> Do we need to backport this for a non-default build with a non-default
> compiler?
If it affected a LTS build I'd say yes, but since that isn't the case
I think it's not necessary.
> > > Signed-off-by: Matthias Kaehlcke <[email protected]>
> > > ---
> > > Changes in v2:
> > > - rebased on drm-tip
> > > - added comment indicating that disabled warnings are clang warnings
> > >
> > > drivers/gpu/drm/i915/Makefile | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 4eee91a3a236..9717c037b582 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -18,6 +18,11 @@ subdir-ccflags-y += $(call cc-disable-warning, type-limits)
> > > subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
> > > subdir-ccflags-y += $(call cc-disable-warning, implicit-fallthrough)
> > > subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable)
> > > +# clang warnings
> > > +subdir-ccflags-y += $(call cc-disable-warning, sign-compare)
>
> Too much mixup in the code to be fixed overnight indeed.
>
> > > +subdir-ccflags-y += $(call cc-disable-warning, sometimes-uninitialized)
>
> Annoyingly it appears that clang has more false positives.
>
> > > +subdir-ccflags-y += $(call cc-disable-warning, unneeded-internal-declaration)
>
> Example? I don't recall this one, so don't know if we should just not
> fix it rather than suppress. I've used ignored-attributes, perhaps that
> was for the same cause.
drivers/gpu/drm/i915/intel_guc_submission.c:183:13: warning: function
'has_doorbell' is not needed and will not be emitted
[-Wunneeded-internal-declaration]
static bool has_doorbell(struct intel_guc_client *client)
The function is only called within a GEM_BUG_ON macro, which does not
evaluate the expression unless CONFIG_DRM_I915_DEBUG_GEM is set.
Instead of disabling the warning it would probably be better to mark
has_doorbell as __maybe_unused.