[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:45 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> 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.

Well, you need to figure out what the correct thing is for integers.
If it's a sign bit, then we have to flip the sign on the immediate
before emission too (since it's all 2's complement, not s-mag). Or if
it's all the high bits. And either way, adjust the target to cope with
the new restrictions, if any.

  -ilia