[Mesa-dev] [PATCH] Add an ir_triop_mid3 expression and lower it to mins and maxs.

[Ian Romanick]

On 04/30/2014 03:21 AM, Petri Latvala wrote:
> On 04/29/2014 09:57 PM, Matt Turner wrote:
>> On Tue, Apr 29, 2014 at 6:01 AM, Petri Latvala
>> <petri.latvala at intel.com> wrote:
>>> For the record, tested this with the following shader:
>>>
>>>
>> Cool. Please submit this as a piglit test.
> 
> Sent to piglit mailing list, with accompanying tests for min3 and max3.
> 
>> Wouldn't it be simpler to detect constant arguments in opt_algebraic
>> and do the optimization there, and just perform the standard lowering
>> here? It seems cleaner to me. I don't think we generate different code
>> based on the arguments in any other lowering pass. 
> 
> I was thinking about that, but ended up optimizing on lowering. My
> reasoning was that if mid3(x, 1, 3) was transformed to min(max(x, 1), 3)
> in opt_algebraic, backends with theoretical native support for mid3
> would then need to recognize that and transform it back to a mid3.

I was thinking we'd emit mid3(x, 1, 3) as the same set of IR as mid3(x,
y, z), but we'd just order the operands so that other optimization
passes could optimize.

Right now we generate:

    max(min(a, b), max(min(a, c), b))

With the values from the bug, this becomes:

    max(min(a, 1), max(min(a, 3), 1))

What we want is

    max(1, min(a, 3))

It's not obvious to me how opt_algebraic could rearrange the expression
tree, for all arrangements of {a, 1, 3}, so that other optimization
passes could do that pruning.  However, that does seem like a reasonable
approach for min3 and max3.

If we think it's possible to make opt_algebraic smart enough to do this
tree rearrangement, then we probably don't need ir_triop_mid3 or the
lowering pass at all.

> (Are there even any GPUs that can do mid3 directly? AMD?)

I believe the Southern Islands GPUs support these instructions natively.
 Certainly whatever GPU is in the Xbox One and PS4 has them.

> Of course, it can be said that it's not a regression as mid3 doesn't get
> to backends as mid3 before this patch either.

I think when the radeonsi driver grows support for emitting these
instructions, they'll want to change st_glsl_to_tgsi to not lower the
instructions away in the firtst place.

> I'll change the patch to do this optimization in opt_algebraic.