[Mesa-dev] [Mesa-stable] [PATCH] i965/fs: Don't emit SEL instructions for type-converting MOVs.

[Francisco Jerez]

Matt Turner <mattst88 at gmail.com> writes:

> On Fri, Mar 24, 2017 at 12:06 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
>>
>>> On Thu, 2017-03-23 at 13:50 -0700, Matt Turner wrote:
>>>> SEL can only convert between a few integer types, which we basically
>>>> never do.
>>>>
>>>> Fixes fs/vs-double-uniform-array-direct-indirect-non-uniform-control-
>>>> flow
>>>> Cc: mesa-stable at lists.freedesktop.org
>>>
>>> I sent a similar but wrong patch (taking only into account the type
>>> size) some time ago, but after discussing it with Curro, the solution
>>> was to fix it inside d2x pass. This is what this patch "i965/fs:
>>> generalize the legalization d2x pass" does.
>>>
>>> I am still working on improving that patch but I expect to have
>>> something soon. If you prefer to land this now, please add my R-b but
>>> you probably want to discuss it with Curro before:
>>>
>>> Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>>>
>>
>> Samuel's d2x patch has the advantage that it will allow the SEL peephole
>> to replace control flow with SEL instructions even where there is a type
>> conversion.  That said this patch shouldn't hurt in mesa-stable in the
>> meantime if we remember to revert it in master when Samuel's patch
>> lands.  Patch is:
>>
>> Acked-by: Francisco Jerez <currojerez at riseup.net>
>
> Oh, I didn't realize that pass was going to handle instructions not
> operating on DF types. That's surprising given its name.

Yeah, he's also sent a patch to rename it.

>
> To confirm: with that pass in place it should be save to do a
> type-converting SEL (on, say, integer sources and a float destination)
> in the IR?
>

Yes, in principle it should be safe for the optimizer to introduce type
conversions of any kind, although at this point the lower_conversions
pass only handles MOV, MOV_INDIRECT and SEL opcodes it should be
straightforward to extend it to handle the type conversion restrictions
of any instruction.

> If that's the case, I'll delay committing this patch until lower_d2x
> is committed, so that we don't have to remember to revert my patch,
> and there's no chance of bugs being fixed on the stable branch but not
> in master.

I guess if you commit it to master already there's no chance of it not
getting fixed in master, the only concern is that we'll end up with both
fixes checked in forever.  Samuel, would you mind including a revert of
this change as PATCH 7.5 of your FP64 series?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170324/5547ff7a/attachment.sig>