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 mka@chromium.org --- 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
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 mka@chromium.org
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 mka@chromium.org
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 mka@chromium.org
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.
Quoting Matthias Kaehlcke (2018-04-30 21:51:45)
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 mka@chromium.org
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.
Hmm, if it is just this one, I would remove the use from intel_guc_submission and move it into selftests/ The single use case inside intel_guc_submission isn't that interesting and I doubt we would miss not having the assert. -Chris
On Mon, Apr 30, 2018 at 10:01:50PM +0100, Chris Wilson wrote:
Quoting Matthias Kaehlcke (2018-04-30 21:51:45)
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 mka@chromium.org
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.
Hmm, if it is just this one, I would remove the use from intel_guc_submission and move it into selftests/ The single use case inside intel_guc_submission isn't that interesting and I doubt we would miss not having the assert.
Could you take care of this? I'm not really familiar with the i915 codebase and might not put it exactly where you want it ;-)
I'd then rebase this patch and leave -Wunneeded-internal-declaration enabled.
dri-devel@lists.freedesktop.org
-
Chris Wilson -
Matthias Kaehlcke