Re: [PATCH] drm: fix lut value extraction function

From: Daniel Vetter <daniel@ffwll.ch>
To: Emil Velikov <emil.l.velikov@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	ML dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: fix lut value extraction function
Date: Wed, 20 Apr 2016 12:52:38 +0200	[thread overview]
Message-ID: <20160420105238.GH2510@phenom.ffwll.local> (raw)
In-Reply-To: <CACvgo52itpYOyS-CxVYkFUKgst90h+2=5iCjPObLJS2kFwgnfw@mail.gmail.com>

On Mon, Apr 18, 2016 at 08:39:00PM +0100, Emil Velikov wrote:
> On 18 April 2016 at 16:53, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Apr 18, 2016 at 03:40:11PM +0100, Emil Velikov wrote:
> >> On 18 April 2016 at 13:36, Daniel Vetter <daniel@ffwll.ch> wrote:
> >> > On Mon, Apr 18, 2016 at 12:09:51PM +0100, Lionel Landwerlin wrote:
> >> >> Ping?
> >> >>
> >> >> On 22/03/16 14:10, Lionel Landwerlin wrote:
> >> >> >When extracting the value at full precision (16 bits), no need to
> >> >> >round the value.
> >> >> >
> >> >> >This was spotted by Jani when running sparse. Unfortunately this fix
> >> >> >doesn't get rid of the warning.
> >> >
> >> > It sounded like no bug, and the patch itself fails to appease sparse. And
> >> > I didn't check what's upsetting sparse itself, so figured "nothing to do
> >> > here until a real fix shows up".
> >> >
> >> According to the C99 standard a left shift with negative value is
> >> undefined. And we're hitting this case at full precision ;-)
> >
> > Well commit message says sparse is still unhappy. So I'm not sure whether
> > the fix is good enough? And the issue with compiler/static checker noise
> > is that we really should aim to shut them up completely, because broken
> > windows and all that (even if it's sometimes a fallacy, I think it applies
> > here).
> Afaics the fix resolves a real bug and the final solution is bug free
> (although one can drop the L form 1UL). If I have to guess I'd say
> that sparse does not realise that the precision cannot be greater than
> 16.
> 
> Quick and easy check is to add an early bail out (if bit_precision >
> 16 return user_input). The compiler will optimise it out anyway (it
> does propagate/fold the constants) the end binary will be fine.
> Another approach is the earlier suggested, switch which will also get
> optimised in the final binary.

Ok, count me convinced ;-) Applied to drm-misc.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel