* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-12 17:17 ` Michel Dänzer
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-12 17:17 ` Michel Dänzer
0 siblings, 0 replies; 38+ 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, Rodrigo Vivi
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ 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
-1 siblings, 0 replies; 38+ 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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 14:37 ` Jani Nikula
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 14:37 ` Jani Nikula
0 siblings, 0 replies; 38+ 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ 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
-1 siblings, 0 replies; 38+ messages in thread
From: Nathan Chancellor @ 2020-02-13 21:48 UTC (permalink / raw)
To: Jani Nikula
Cc: Michel Dänzer, clang-built-linux, intel-gfx, linux-kernel,
dri-devel
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 21:48 ` Nathan Chancellor
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 21:48 ` Nathan Chancellor
0 siblings, 0 replies; 38+ 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ 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
-1 siblings, 0 replies; 38+ messages in thread
From: Jani Nikula @ 2020-02-13 22:05 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Michel Dänzer, clang-built-linux, intel-gfx, linux-kernel,
dri-devel
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 22:05 ` Jani Nikula
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 22:05 ` Jani Nikula
0 siblings, 0 replies; 38+ 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
2020-02-12 17:17 ` Michel Dänzer
(?)
@ 2020-02-13 22:43 ` Nick Desaulniers
-1 siblings, 0 replies; 38+ messages in thread
From: Nick Desaulniers @ 2020-02-13 22:43 UTC (permalink / raw)
To: Michel Dänzer
Cc: Nathan Chancellor, intel-gfx, LKML, clang-built-linux, dri-devel,
Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 22:43 ` Nick Desaulniers
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 22:43 ` Nick Desaulniers
0 siblings, 0 replies; 38+ 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, Rodrigo Vivi,
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [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
-1 siblings, 0 replies; 38+ messages in thread
From: Nathan Chancellor @ 2020-02-13 23:27 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Michel Dänzer, intel-gfx, LKML, clang-built-linux,
dri-devel, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Intel-gfx] [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 23:27 ` Nathan Chancellor
0 siblings, 0 replies; 38+ 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] 38+ messages in thread
* Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
@ 2020-02-13 23:27 ` Nathan Chancellor
0 siblings, 0 replies; 38+ 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,
Rodrigo Vivi, 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 38+ messages in thread