linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nathan Chancellor <natechancellor@gmail.com>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	clang-built-linux@googlegroups.com,
	dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare
Date: Wed, 12 Feb 2020 10:07:34 -0700	[thread overview]
Message-ID: <20200212170734.GA16396@ubuntu-m2-xlarge-x86> (raw)
In-Reply-To: <f3a6346b-2abf-0b6a-3d84-66e12f700b2b@daenzer.net>

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

  reply	other threads:[~2020-02-12 17:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11  5:08 [PATCH] " Nathan Chancellor
2020-02-11  6:13 ` [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 [this message]
2020-02-12 17:17           ` Michel Dänzer
2020-02-13 14:37             ` [Intel-gfx] " 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200212170734.GA16396@ubuntu-m2-xlarge-x86 \
    --to=natechancellor@gmail.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michel@daenzer.net \
    --cc=rodrigo.vivi@intel.com \
    --subject='Re: [PATCH v2] drm/i915: Disable -Wtautological-constant-out-of-range-compare' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).