* [Intel-gfx] [PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare @ 2020-02-11 5:08 Nathan Chancellor 2020-02-11 5:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Nathan Chancellor @ 2020-02-11 5:08 UTC (permalink / raw) To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi Cc: Nathan Chancellor, clang-built-linux, intel-gfx, linux-kernel, dri-devel A recent commit in clang added -Wtautological-compare to -Wall, which is enabled for i915 so we see the following warning: ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: result of comparison of constant 576460752303423487 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] if (unlikely(remain > N_RELOC(ULONG_MAX))) ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ This warning only happens on x86_64 but that check is relevant for 32-bit x86 so we cannot remove it. -Wtautological-compare on a whole has good warnings but this one is not really relevant for the kernel because of all of the different configurations that are used to build the kernel. When -Wtautological-compare is enabled for the kernel, this option will remain disabled so do that for i915 now. Link: https://github.com/ClangBuiltLinux/linux/issues/778 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/gpu/drm/i915/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 38df01c23176..55dbcca179c7 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -21,6 +21,7 @@ subdir-ccflags-y += $(call cc-disable-warning, unused-but-set-variable) 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, initializer-overrides) +subdir-ccflags-y += $(call cc-disable-warning, tautological-constant-out-of-range-compare) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror # Fine grained warnings disable -- 2.25.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-11 5:08 [Intel-gfx] [PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare Nathan Chancellor @ 2020-02-11 5:29 ` Patchwork 2020-02-11 6:13 ` [Intel-gfx] [PATCH v2] " Nathan Chancellor 2020-02-11 6:58 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Disable -Wtautological-constant-out-of-range-compare (rev2) Patchwork 2 siblings, 0 replies; 14+ messages in thread From: Patchwork @ 2020-02-11 5:29 UTC (permalink / raw) To: Nathan Chancellor; +Cc: intel-gfx == Series Details == Series: drm/i915: Disable -Wtautological-constant-out-of-range-compare URL : https://patchwork.freedesktop.org/series/73271/ State : failure == Summary == Applying: drm/i915: Disable -Wtautological-constant-out-of-range-compare error: sha1 information is lacking or useless (drivers/gpu/drm/i915/Makefile). error: could not build fake ancestor hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 drm/i915: Disable -Wtautological-constant-out-of-range-compare When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-11 5:08 [Intel-gfx] [PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare Nathan Chancellor 2020-02-11 5:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork @ 2020-02-11 6:13 ` Nathan Chancellor 2020-02-11 9:41 ` Michel Dänzer 2020-02-11 6:58 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Disable -Wtautological-constant-out-of-range-compare (rev2) Patchwork 2 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2020-02-11 6:13 UTC (permalink / raw) To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi Cc: Nathan Chancellor, clang-built-linux, intel-gfx, linux-kernel, dri-devel A recent commit in clang added -Wtautological-compare to -Wall, which is enabled for i915 so we see the following warning: ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: result of comparison of constant 576460752303423487 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] if (unlikely(remain > N_RELOC(ULONG_MAX))) ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ This warning only happens on x86_64 but that check is relevant for 32-bit x86 so we cannot remove it. -Wtautological-compare on a whole has good warnings but this one is not really relevant for the kernel because of all of the different configurations that are used to build the kernel. When -Wtautological-compare is enabled for the kernel, this option will remain disabled so do that for i915 now. Link: https://github.com/ClangBuiltLinux/linux/issues/778 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: https://lore.kernel.org/lkml/20200211050808.29463-1-natechancellor@gmail.com/ * Fix patch application due to basing on a local tree that had -Wuninitialized turned on. Can confirm that patch applies on latest -next now. drivers/gpu/drm/i915/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index b8c5f8934dbd..159355eb43a9 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -22,6 +22,7 @@ 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, initializer-overrides) subdir-ccflags-y += $(call cc-disable-warning, uninitialized) +subdir-ccflags-y += $(call cc-disable-warning, tautological-constant-out-of-range-compare) subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror # Fine grained warnings disable -- 2.25.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-11 6:13 ` [Intel-gfx] [PATCH v2] " Nathan Chancellor @ 2020-02-11 9:41 ` Michel Dänzer 2020-02-11 20:39 ` Nathan Chancellor 0 siblings, 1 reply; 14+ messages in thread From: Michel Dänzer @ 2020-02-11 9:41 UTC (permalink / raw) To: Nathan Chancellor, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi Cc: clang-built-linux, intel-gfx, linux-kernel, dri-devel On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > A recent commit in clang added -Wtautological-compare to -Wall, which is > enabled for i915 so we see the following warning: > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > result of comparison of constant 576460752303423487 with expression of > type 'unsigned int' is always false > [-Wtautological-constant-out-of-range-compare] > if (unlikely(remain > N_RELOC(ULONG_MAX))) > ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > > This warning only happens on x86_64 but that check is relevant for > 32-bit x86 so we cannot remove it. That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value in both cases, and remain is a 32-bit value in both cases. How can it be larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-11 9:41 ` Michel Dänzer @ 2020-02-11 20:39 ` Nathan Chancellor 2020-02-12 8:52 ` Michel Dänzer 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2020-02-11 20:39 UTC (permalink / raw) To: Michel Dänzer; +Cc: intel-gfx, linux-kernel, clang-built-linux, dri-devel On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > > A recent commit in clang added -Wtautological-compare to -Wall, which is > > enabled for i915 so we see the following warning: > > > > ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > > result of comparison of constant 576460752303423487 with expression of > > type 'unsigned int' is always false > > [-Wtautological-constant-out-of-range-compare] > > if (unlikely(remain > N_RELOC(ULONG_MAX))) > > ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > > > > This warning only happens on x86_64 but that check is relevant for > > 32-bit x86 so we cannot remove it. > > That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > in both cases, and remain is a 32-bit value in both cases. How can it be > larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > Hi Michel, Can't this condition be true when UINT_MAX == ULONG_MAX? clang does not warn on a 32-bit x86 build from what I remember. Honestly, my understanding of overflow is pretty shoddy, this is mostly based on what I have heard from others. I sent a patch trying to remove that check but had it rejected: https://lore.kernel.org/lkml/20191123195321.41305-1-natechancellor@gmail.com/ Cheers, Nathan _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-11 20:39 ` Nathan Chancellor @ 2020-02-12 8:52 ` Michel Dänzer 2020-02-12 17:07 ` Nathan Chancellor 0 siblings, 1 reply; 14+ messages in thread From: Michel Dänzer @ 2020-02-12 8:52 UTC (permalink / raw) To: Nathan Chancellor; +Cc: clang-built-linux, intel-gfx, linux-kernel, dri-devel On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: >> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >>> A recent commit in clang added -Wtautological-compare to -Wall, which is >>> enabled for i915 so we see the following warning: >>> >>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >>> result of comparison of constant 576460752303423487 with expression of >>> type 'unsigned int' is always false >>> [-Wtautological-constant-out-of-range-compare] >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ >>> >>> This warning only happens on x86_64 but that check is relevant for >>> 32-bit x86 so we cannot remove it. >> >> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value >> in both cases, and remain is a 32-bit value in both cases. How can it be >> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? >> > > Hi Michel, > > Can't this condition be true when UINT_MAX == ULONG_MAX? Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. Anyway, this suggests a possible better solution: #if UINT_MAX == ULONG_MAX if (unlikely(remain > N_RELOC(ULONG_MAX))) return -EINVAL; #endif Or if that can't be used for some reason, something like if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) return -EINVAL; should silence the warning. Either of these should be better than completely disabling the warning for the whole file. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-12 8:52 ` Michel Dänzer @ 2020-02-12 17:07 ` Nathan Chancellor 2020-02-12 17:17 ` Michel Dänzer 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2020-02-12 17:07 UTC (permalink / raw) To: Michel Dänzer; +Cc: clang-built-linux, intel-gfx, linux-kernel, dri-devel On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > > On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > >> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > >>> A recent commit in clang added -Wtautological-compare to -Wall, which is > >>> enabled for i915 so we see the following warning: > >>> > >>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > >>> result of comparison of constant 576460752303423487 with expression of > >>> type 'unsigned int' is always false > >>> [-Wtautological-constant-out-of-range-compare] > >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > >>> > >>> This warning only happens on x86_64 but that check is relevant for > >>> 32-bit x86 so we cannot remove it. > >> > >> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > >> in both cases, and remain is a 32-bit value in both cases. How can it be > >> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > >> > > > > Hi Michel, > > > > Can't this condition be true when UINT_MAX == ULONG_MAX? > > Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > > > Anyway, this suggests a possible better solution: > > #if UINT_MAX == ULONG_MAX > if (unlikely(remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > #endif > > > Or if that can't be used for some reason, something like > > if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > return -EINVAL; > > should silence the warning. I do like this one better than the former. > > > Either of these should be better than completely disabling the warning > for the whole file. Normally, I would agree but I am currently planning to leave -Wtautological-constant-out-of-range-compare disabled when I turn on -Wtautological-compare for the whole kernel because there are plenty of locations in the kernel where these kind of checks depend on various kernel configuration options and the general attitude of kernel developers is that this particular warning is not really helpful for that reason. I'll see if there is a general consensus before moving further since I know i915 turns on a bunch of extra warnings from the rest of the kernel (hence why we are in this situation). Cheers, Nathan _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-12 17:07 ` Nathan Chancellor @ 2020-02-12 17:17 ` Michel Dänzer 2020-02-13 14:37 ` Jani Nikula 2020-02-13 22:43 ` Nick Desaulniers 0 siblings, 2 replies; 14+ messages in thread From: Michel Dänzer @ 2020-02-12 17:17 UTC (permalink / raw) To: Nathan Chancellor; +Cc: clang-built-linux, intel-gfx, linux-kernel, dri-devel On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: >> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: >>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: >>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is >>>>> enabled for i915 so we see the following warning: >>>>> >>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >>>>> result of comparison of constant 576460752303423487 with expression of >>>>> type 'unsigned int' is always false >>>>> [-Wtautological-constant-out-of-range-compare] >>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ >>>>> >>>>> This warning only happens on x86_64 but that check is relevant for >>>>> 32-bit x86 so we cannot remove it. >>>> >>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value >>>> in both cases, and remain is a 32-bit value in both cases. How can it be >>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? >>>> >>> >>> Hi Michel, >>> >>> Can't this condition be true when UINT_MAX == ULONG_MAX? >> >> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. >> >> >> Anyway, this suggests a possible better solution: >> >> #if UINT_MAX == ULONG_MAX >> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> return -EINVAL; >> #endif >> >> >> Or if that can't be used for some reason, something like >> >> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) >> return -EINVAL; >> >> should silence the warning. > > I do like this one better than the former. FWIW, one downside of this one compared to all alternatives (presumably) is that it might end up generating actual code even on 64-bit, which always ends up skipping the return. -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-12 17:17 ` Michel Dänzer @ 2020-02-13 14:37 ` Jani Nikula 2020-02-13 21:48 ` Nathan Chancellor 2020-02-13 22:43 ` Nick Desaulniers 1 sibling, 1 reply; 14+ messages in thread From: Jani Nikula @ 2020-02-13 14:37 UTC (permalink / raw) To: Michel Dänzer, Nathan Chancellor Cc: clang-built-linux, intel-gfx, linux-kernel, dri-devel On Wed, 12 Feb 2020, Michel Dänzer <michel@daenzer.net> wrote: > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: >>>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: >>>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >>>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is >>>>>> enabled for i915 so we see the following warning: >>>>>> >>>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >>>>>> result of comparison of constant 576460752303423487 with expression of >>>>>> type 'unsigned int' is always false >>>>>> [-Wtautological-constant-out-of-range-compare] >>>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >>>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ >>>>>> >>>>>> This warning only happens on x86_64 but that check is relevant for >>>>>> 32-bit x86 so we cannot remove it. >>>>> >>>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value >>>>> in both cases, and remain is a 32-bit value in both cases. How can it be >>>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? >>>>> >>>> >>>> Hi Michel, >>>> >>>> Can't this condition be true when UINT_MAX == ULONG_MAX? >>> >>> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. >>> >>> >>> Anyway, this suggests a possible better solution: >>> >>> #if UINT_MAX == ULONG_MAX >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >>> return -EINVAL; >>> #endif >>> >>> >>> Or if that can't be used for some reason, something like >>> >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) >>> return -EINVAL; >>> >>> should silence the warning. >> >> I do like this one better than the former. > > FWIW, one downside of this one compared to all alternatives (presumably) > is that it might end up generating actual code even on 64-bit, which > always ends up skipping the return. I like this better than the UINT_MAX == ULONG_MAX comparison because that creates a dependency on the type of remain. Then again, a sufficiently clever compiler could see through the cast, and flag the warning anyway... BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-13 14:37 ` Jani Nikula @ 2020-02-13 21:48 ` Nathan Chancellor 2020-02-13 22:05 ` Jani Nikula 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2020-02-13 21:48 UTC (permalink / raw) To: Jani Nikula Cc: clang-built-linux, Michel Dänzer, intel-gfx, dri-devel, linux-kernel On Thu, Feb 13, 2020 at 04:37:15PM +0200, Jani Nikula wrote: > On Wed, 12 Feb 2020, Michel Dänzer <michel@daenzer.net> wrote: > > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > >>>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > >>>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > >>>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is > >>>>>> enabled for i915 so we see the following warning: > >>>>>> > >>>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > >>>>>> result of comparison of constant 576460752303423487 with expression of > >>>>>> type 'unsigned int' is always false > >>>>>> [-Wtautological-constant-out-of-range-compare] > >>>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > >>>>>> > >>>>>> This warning only happens on x86_64 but that check is relevant for > >>>>>> 32-bit x86 so we cannot remove it. > >>>>> > >>>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > >>>>> in both cases, and remain is a 32-bit value in both cases. How can it be > >>>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > >>>>> > >>>> > >>>> Hi Michel, > >>>> > >>>> Can't this condition be true when UINT_MAX == ULONG_MAX? > >>> > >>> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > >>> > >>> > >>> Anyway, this suggests a possible better solution: > >>> > >>> #if UINT_MAX == ULONG_MAX > >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>> return -EINVAL; > >>> #endif > >>> > >>> > >>> Or if that can't be used for some reason, something like > >>> > >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > >>> return -EINVAL; > >>> > >>> should silence the warning. > >> > >> I do like this one better than the former. > > > > FWIW, one downside of this one compared to all alternatives (presumably) > > is that it might end up generating actual code even on 64-bit, which > > always ends up skipping the return. > > I like this better than the UINT_MAX == ULONG_MAX comparison because > that creates a dependency on the type of remain. > > Then again, a sufficiently clever compiler could see through the cast, > and flag the warning anyway... Would you prefer a patch that adds that cast rather than silencing the warning outright? It does appear to work for clang. Cheers, Nathan _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-13 21:48 ` Nathan Chancellor @ 2020-02-13 22:05 ` Jani Nikula 0 siblings, 0 replies; 14+ messages in thread From: Jani Nikula @ 2020-02-13 22:05 UTC (permalink / raw) To: Nathan Chancellor Cc: clang-built-linux, Michel Dänzer, intel-gfx, dri-devel, linux-kernel On Thu, 13 Feb 2020, Nathan Chancellor <natechancellor@gmail.com> wrote: > On Thu, Feb 13, 2020 at 04:37:15PM +0200, Jani Nikula wrote: >> On Wed, 12 Feb 2020, Michel Dänzer <michel@daenzer.net> wrote: >> > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: >> >> On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: >> >>> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: >> >>>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: >> >>>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: >> >>>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is >> >>>>>> enabled for i915 so we see the following warning: >> >>>>>> >> >>>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: >> >>>>>> result of comparison of constant 576460752303423487 with expression of >> >>>>>> type 'unsigned int' is always false >> >>>>>> [-Wtautological-constant-out-of-range-compare] >> >>>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> >>>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ >> >>>>>> >> >>>>>> This warning only happens on x86_64 but that check is relevant for >> >>>>>> 32-bit x86 so we cannot remove it. >> >>>>> >> >>>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value >> >>>>> in both cases, and remain is a 32-bit value in both cases. How can it be >> >>>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? >> >>>>> >> >>>> >> >>>> Hi Michel, >> >>>> >> >>>> Can't this condition be true when UINT_MAX == ULONG_MAX? >> >>> >> >>> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. >> >>> >> >>> >> >>> Anyway, this suggests a possible better solution: >> >>> >> >>> #if UINT_MAX == ULONG_MAX >> >>> if (unlikely(remain > N_RELOC(ULONG_MAX))) >> >>> return -EINVAL; >> >>> #endif >> >>> >> >>> >> >>> Or if that can't be used for some reason, something like >> >>> >> >>> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) >> >>> return -EINVAL; >> >>> >> >>> should silence the warning. >> >> >> >> I do like this one better than the former. >> > >> > FWIW, one downside of this one compared to all alternatives (presumably) >> > is that it might end up generating actual code even on 64-bit, which >> > always ends up skipping the return. >> >> I like this better than the UINT_MAX == ULONG_MAX comparison because >> that creates a dependency on the type of remain. >> >> Then again, a sufficiently clever compiler could see through the cast, >> and flag the warning anyway... > > Would you prefer a patch that adds that cast rather than silencing the > warning outright? It does appear to work for clang. I'd take the cast. If that fails for whatever reason, per-file CFLAGS_gem/i915_gem_execbuffer.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare) over subdir-ccflags-y would be preferrable I think. BR, Jani. > > Cheers, > Nathan -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-12 17:17 ` Michel Dänzer 2020-02-13 14:37 ` Jani Nikula @ 2020-02-13 22:43 ` Nick Desaulniers 2020-02-13 23:27 ` Nathan Chancellor 1 sibling, 1 reply; 14+ messages in thread From: Nick Desaulniers @ 2020-02-13 22:43 UTC (permalink / raw) To: Michel Dänzer Cc: intel-gfx, LKML, dri-devel, clang-built-linux, Nathan Chancellor On Wed, Feb 12, 2020 at 9:17 AM Michel Dänzer <michel@daenzer.net> wrote: > > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > > On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > >> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > >>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > >>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > >>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is > >>>>> enabled for i915 so we see the following warning: > >>>>> > >>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > >>>>> result of comparison of constant 576460752303423487 with expression of > >>>>> type 'unsigned int' is always false > >>>>> [-Wtautological-constant-out-of-range-compare] > >>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > >>>>> > >>>>> This warning only happens on x86_64 but that check is relevant for > >>>>> 32-bit x86 so we cannot remove it. > >>>> > >>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > >>>> in both cases, and remain is a 32-bit value in both cases. How can it be > >>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > >>>> > >>> > >>> Hi Michel, > >>> > >>> Can't this condition be true when UINT_MAX == ULONG_MAX? > >> > >> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > >> > >> > >> Anyway, this suggests a possible better solution: > >> > >> #if UINT_MAX == ULONG_MAX > >> if (unlikely(remain > N_RELOC(ULONG_MAX))) > >> return -EINVAL; > >> #endif > >> > >> > >> Or if that can't be used for some reason, something like > >> > >> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > >> return -EINVAL; > >> > >> should silence the warning. > > > > I do like this one better than the former. > > FWIW, one downside of this one compared to all alternatives (presumably) > is that it might end up generating actual code even on 64-bit, which > always ends up skipping the return. The warning is pointing out that the conditional is always false, which is correct on 64b. The check is only active for 32b. https://godbolt.org/z/oQrgT_ The cast silences the warning for 64b. (Note that GCC and Clang also generate precisely the same instruction sequences in my example, just GCC doesn't warn on such tautologies). -- Thanks, ~Nick Desaulniers _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare 2020-02-13 22:43 ` Nick Desaulniers @ 2020-02-13 23:27 ` Nathan Chancellor 0 siblings, 0 replies; 14+ messages in thread From: Nathan Chancellor @ 2020-02-13 23:27 UTC (permalink / raw) To: Nick Desaulniers Cc: Michel Dänzer, LKML, dri-devel, clang-built-linux, intel-gfx On Thu, Feb 13, 2020 at 02:43:21PM -0800, Nick Desaulniers wrote: > On Wed, Feb 12, 2020 at 9:17 AM Michel Dänzer <michel@daenzer.net> wrote: > > > > On 2020-02-12 6:07 p.m., Nathan Chancellor wrote: > > > On Wed, Feb 12, 2020 at 09:52:52AM +0100, Michel Dänzer wrote: > > >> On 2020-02-11 9:39 p.m., Nathan Chancellor wrote: > > >>> On Tue, Feb 11, 2020 at 10:41:48AM +0100, Michel Dänzer wrote: > > >>>> On 2020-02-11 7:13 a.m., Nathan Chancellor wrote: > > >>>>> A recent commit in clang added -Wtautological-compare to -Wall, which is > > >>>>> enabled for i915 so we see the following warning: > > >>>>> > > >>>>> ../drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c:1485:22: warning: > > >>>>> result of comparison of constant 576460752303423487 with expression of > > >>>>> type 'unsigned int' is always false > > >>>>> [-Wtautological-constant-out-of-range-compare] > > >>>>> if (unlikely(remain > N_RELOC(ULONG_MAX))) > > >>>>> ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ > > >>>>> > > >>>>> This warning only happens on x86_64 but that check is relevant for > > >>>>> 32-bit x86 so we cannot remove it. > > >>>> > > >>>> That's suprising. AFAICT N_RELOC(ULONG_MAX) works out to the same value > > >>>> in both cases, and remain is a 32-bit value in both cases. How can it be > > >>>> larger than N_RELOC(ULONG_MAX) on 32-bit (but not on 64-bit)? > > >>>> > > >>> > > >>> Hi Michel, > > >>> > > >>> Can't this condition be true when UINT_MAX == ULONG_MAX? > > >> > > >> Oh, right, I think I was wrongly thinking long had 64 bits even on 32-bit. > > >> > > >> > > >> Anyway, this suggests a possible better solution: > > >> > > >> #if UINT_MAX == ULONG_MAX > > >> if (unlikely(remain > N_RELOC(ULONG_MAX))) > > >> return -EINVAL; > > >> #endif > > >> > > >> > > >> Or if that can't be used for some reason, something like > > >> > > >> if (unlikely((unsigned long)remain > N_RELOC(ULONG_MAX))) > > >> return -EINVAL; > > >> > > >> should silence the warning. > > > > > > I do like this one better than the former. > > > > FWIW, one downside of this one compared to all alternatives (presumably) > > is that it might end up generating actual code even on 64-bit, which > > always ends up skipping the return. > > The warning is pointing out that the conditional is always false, > which is correct on 64b. The check is only active for 32b. > https://godbolt.org/z/oQrgT_ > The cast silences the warning for 64b. (Note that GCC and Clang also > generate precisely the same instruction sequences in my example, just > GCC doesn't warn on such tautologies). Thanks for confirming! I'll send a patch to add the cast later tonight. Cheers, Nathan _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Disable -Wtautological-constant-out-of-range-compare (rev2) 2020-02-11 5:08 [Intel-gfx] [PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare Nathan Chancellor 2020-02-11 5:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork 2020-02-11 6:13 ` [Intel-gfx] [PATCH v2] " Nathan Chancellor @ 2020-02-11 6:58 ` Patchwork 2 siblings, 0 replies; 14+ messages in thread From: Patchwork @ 2020-02-11 6:58 UTC (permalink / raw) To: Nathan Chancellor; +Cc: intel-gfx == Series Details == Series: drm/i915: Disable -Wtautological-constant-out-of-range-compare (rev2) URL : https://patchwork.freedesktop.org/series/73271/ State : failure == Summary == CI Bug Log - changes from CI_DRM_7905 -> Patchwork_16515 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_16515 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_16515, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/index.html Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_16515: ### IGT changes ### #### Possible regressions #### * igt@i915_selftest@live_blt: - fi-hsw-4770r: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7905/fi-hsw-4770r/igt@i915_selftest@live_blt.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/fi-hsw-4770r/igt@i915_selftest@live_blt.html * igt@i915_selftest@live_objects: - fi-bwr-2160: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7905/fi-bwr-2160/igt@i915_selftest@live_objects.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/fi-bwr-2160/igt@i915_selftest@live_objects.html Known issues ------------ Here are the changes found in Patchwork_16515 that come from known issues: ### IGT changes ### #### Possible fixes #### * igt@gem_close_race@basic-threads: - fi-hsw-peppy: [INCOMPLETE][5] ([i915#694] / [i915#816]) -> [PASS][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7905/fi-hsw-peppy/igt@gem_close_race@basic-threads.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/fi-hsw-peppy/igt@gem_close_race@basic-threads.html * igt@i915_selftest@live_gem_contexts: - fi-cfl-8700k: [INCOMPLETE][7] ([i915#424]) -> [PASS][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7905/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424 [i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694 [i915#816]: https://gitlab.freedesktop.org/drm/intel/issues/816 Participating hosts (46 -> 40) ------------------------------ Additional (4): fi-skl-lmem fi-glk-dsi fi-snb-2520m fi-kbl-r Missing (10): fi-ilk-m540 fi-bsw-n3050 fi-byt-j1900 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-blb-e6850 fi-byt-clapper fi-bsw-nick Build changes ------------- * CI: CI-20190529 -> None * Linux: CI_DRM_7905 -> Patchwork_16515 CI-20190529: 20190529 CI_DRM_7905: db98da3dd757a19dbaaeaef8640276fe7be2fc4e @ git://anongit.freedesktop.org/gfx-ci/linux IGT_5433: 6a96c17f3a1b4e1f90b1a0b0ce42a7219875d1a4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools Patchwork_16515: 2b06b9a9a6d6785235ca559832db83a0e69928ff @ git://anongit.freedesktop.org/gfx-ci/linux == Linux commits == 2b06b9a9a6d6 drm/i915: Disable -Wtautological-constant-out-of-range-compare == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16515/index.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-02-13 23:27 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-11 5:08 [Intel-gfx] [PATCH] drm/i915: Disable -Wtautological-constant-out-of-range-compare Nathan Chancellor 2020-02-11 5:29 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork 2020-02-11 6:13 ` [Intel-gfx] [PATCH v2] " Nathan Chancellor 2020-02-11 9:41 ` Michel Dänzer 2020-02-11 20:39 ` Nathan Chancellor 2020-02-12 8:52 ` Michel Dänzer 2020-02-12 17:07 ` Nathan Chancellor 2020-02-12 17:17 ` Michel Dänzer 2020-02-13 14:37 ` Jani Nikula 2020-02-13 21:48 ` Nathan Chancellor 2020-02-13 22:05 ` Jani Nikula 2020-02-13 22:43 ` Nick Desaulniers 2020-02-13 23:27 ` Nathan Chancellor 2020-02-11 6:58 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Disable -Wtautological-constant-out-of-range-compare (rev2) Patchwork
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).