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

[Samuel Pitoiset]

On 06/28/2016 04:38 PM, Ilia Mirkin wrote:
> 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?

Yes, it does not do that.

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

Okay, your call. :-)

>

-- 
-Samuel