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.latv...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev