[Mesa-stable] [Mesa-dev] [PATCH 1/2] gm107/ir: make use of FADD32I for all immediates

[Ilia Mirkin]

On Tue, Jun 28, 2016 at 10:27 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 06/28/2016 04:23 PM, Ilia Mirkin wrote:
>>
>> On Tue, Jun 28, 2016 at 10:21 AM, Samuel Pitoiset
>> <samuel.pitoiset at gmail.com> wrote:
>>>
>>> On 06/28/2016 04:15 PM, Ilia Mirkin wrote:
>>>>
>>>>
>>>> Again, what problem was this patch trying to solve?
>>>
>>>
>>>
>>> The problem is that FADD can only emits 19-bits but longIMMD() will
>>> return
>>> false because it only checks for the high 12-bits.
>>>
>>> I don't know if you saw my messages on IRC but I found some other issues
>>> with longIMMD() and emitIMMD().
>>
>>
>> Nope, it will emit 19 bits and then the 20th (high aka sign) bit as
>> well, just to a different location. [And the bottom 12 bits are
>> guaranteed to be 0.]
>>
>> What's a specific example that you think it doesn't emit correctly?
>
>
> I don't have any shaders which hit that issue, but I think it's similar to
> the fix I did for IMUL32I. The immediate value was 0xf4240 in that specific
> case, and IMUL emitted 0x74240 instead... because the sign bit was used to
> emit the NEG modifier.

Right, which isn't the same thing for ints, but is the same thing for
floats. For integer immediates, it's also the low 20 bits, not the
high 20 bits. And I believe that the condition should be ensuring that
all 12 of the high bits are the same. But perhaps it doesn't properly
check that those 12 bits have the same value as the 20th bit?

Anyways... if it ain't broken, don't fix it. Doesn't sound like FADD
emission is broken in any way - let's not fix it.