[PATCH] drm: fix lut value extraction function

Emil Velikov emil.l.velikov at gmail.com
Mon Apr 18 19:39:00 UTC 2016


On 18 April 2016 at 16:53, Daniel Vetter <daniel at 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 at 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.

Emil


More information about the dri-devel mailing list