[PATCH v2] drm/i915: Disable some extra clang warnings

[PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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
-- 
2.16.2.804.g6dcf76e118-goog

[PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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
-- 
2.16.2.804.g6dcf76e118-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.SPARSE: warning for drm/i915: Disable some extra clang warnings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable some extra clang warnings (rev2)
URL   : https://patchwork.freedesktop.org/series/40145/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Disable some extra clang warnings
-
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    expected void [noderef] <asn:4>**slot
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    expected void **slot
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    expected void **slot
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    expected void **slot
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    got void [noderef] <asn:4>**
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    got void [noderef] <asn:4>**
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    got void [noderef] <asn:4>**
+drivers/gpu/drm/i915/gvt/gtt.c:661:9:    got void **slot
+drivers/gpu/drm/i915/gvt/gtt.c:661:9: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gvt/gtt.c:661:9: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gvt/gtt.c:661:9: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gvt/gtt.c:661:9: warning: incorrect type in assignment (different address spaces)
+drivers/gpu/drm/i915/gvt/gtt.c:662:45:    expected void [noderef] <asn:4>**slot
+drivers/gpu/drm/i915/gvt/gtt.c:662:45:    got void **slot
+drivers/gpu/drm/i915/gvt/gtt.c:662:45: warning: incorrect type in argument 1 (different address spaces)
+drivers/gpu/drm/i915/gvt/mmio.c:255:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/gvt/mmio.c:256:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1365:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1423:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_request.c:304:13: error: undefined identifier '__builtin_add_overflow_p'
+drivers/gpu/drm/i915/i915_request.c:304:13: warning: call with no type!
+drivers/gpu/drm/i915/selftests/i915_syncmap.c:80:54: warning: dubious: x | !y
+./include/linux/relay.h:258:34:    expected void const [noderef] <asn:3>*__vpp_verify
+./include/linux/relay.h:258:34:    expected void const [noderef] <asn:3>*__vpp_verify
+./include/linux/relay.h:258:34:    got struct rchan_buf **<noident>
+./include/linux/relay.h:258:34:    got struct rchan_buf **<noident>
+./include/linux/relay.h:258:34: warning: incorrect type in initializer (different address spaces)
+./include/linux/relay.h:258:34: warning: incorrect type in initializer (different address spaces)
+./include/linux/relay.h:269:9: warning: dereference of noderef expression
+./include/linux/relay.h:269:9: warning: dereference of noderef expression

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Disable some extra clang warnings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable some extra clang warnings (rev2)
URL   : https://patchwork.freedesktop.org/series/40145/
State : success

== Summary ==

Series 40145v2 drm/i915: Disable some extra clang warnings
https://patchwork.freedesktop.org/api/1.0/series/40145/revisions/2/mbox/

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:436s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:444s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:385s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:550s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:296s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:515s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:516s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:502s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:410s
fi-cfl-s2        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:586s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:509s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:532s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:421s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:312s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:536s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:406s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:429s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:653s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:438s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:533s
fi-skl-6700hq    total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:541s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:503s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:485s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:441s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:587s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:396s

260af42eeff094e4768265a6ec8bbcb29b87e9a0 drm-tip: 2018y-03m-19d-17h-15m-08s UTC integration manifest
6f7316e384a1 drm/i915: Disable some extra clang warnings

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8403/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915: Disable some extra clang warnings (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable some extra clang warnings (rev2)
URL   : https://patchwork.freedesktop.org/series/40145/
State : warning

== Summary ==

---- Possible new issues:

Test drv_suspend:
        Subgroup forcewake:
                pass       -> SKIP       (shard-snb)
Test kms_vblank:
        Subgroup pipe-a-ts-continuation-suspend:
                pass       -> SKIP       (shard-snb)

---- Known issues:

Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#102887
        Subgroup flip-vs-absolute-wf_vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#100368
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-apl) fdo#101623
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912 +1
Test kms_sysfs_edid_timing:
                warn       -> PASS       (shard-apl) fdo#100047

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3478 pass:1815 dwarn:1   dfail:0   fail:7   skip:1655 time:13086s
shard-hsw        total:3478 pass:1767 dwarn:1   dfail:0   fail:2   skip:1707 time:11721s
shard-snb        total:3478 pass:1356 dwarn:1   dfail:0   fail:2   skip:2119 time:7194s
Blacklisted hosts:
shard-kbl        total:3386 pass:1863 dwarn:31  dfail:0   fail:9   skip:1482 time:9607s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8403/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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

Re: [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Chris Wilson]

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

Re: [PATCH v2] drm/i915: Disable some extra clang warnings

[Chris Wilson]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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.

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Chris Wilson]

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
 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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.

Re: [Intel-gfx] [PATCH v2] drm/i915: Disable some extra clang warnings

[Matthias Kaehlcke]

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 mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel