[Mesa-dev] [PATCH] r600g: fix regression since UCMP change

[Connor Abbott]

On Tue, Dec 9, 2014 at 12:44 AM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 09.12.2014 um 02:31 schrieb Dave Airlie:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> Since d8da6deceadf5e48201d848b7061dad17a5b7cac where the
>> state tracker started using UCMP on cayman a number of tests
>> regressed.
>>
>> this seems to be r600g is doing CNDGE_INT for UCMP which is >= 0,
>> we should be doing CNDE_INT with reverse arguments.
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  src/gallium/drivers/r600/r600_shader.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
>> index 0b988df..28137e1 100644
>> --- a/src/gallium/drivers/r600/r600_shader.c
>> +++ b/src/gallium/drivers/r600/r600_shader.c
>> @@ -6082,7 +6082,7 @@ static int tgsi_ucmp(struct r600_shader_ctx *ctx)
>>                       continue;
>>
>>               memset(&alu, 0, sizeof(struct r600_bytecode_alu));
>> -             alu.op = ALU_OP3_CNDGE_INT;
>> +             alu.op = ALU_OP3_CNDE_INT;
>>               r600_bytecode_src(&alu.src[0], &ctx->src[0], i);
>>               r600_bytecode_src(&alu.src[1], &ctx->src[2], i);
>>               r600_bytecode_src(&alu.src[2], &ctx->src[1], i);
>>
>
> Oh, the state tracker used UCMP before (for triop_csel), which is also
> why I didn't even think about checking drivers if they do the right
> thing... But possibly only with true booleans, so you'd have got only 0
> and -1 as input (though I have no idea again if you'd actually see such
> ops at all from glsl). So maybe it was the optimization to try to avoid
> the extra comparison if it was a equal/not equal comparison against 0
> which caused this indeed :-).

We use ir_triop_csel for implementing two different GLSL builtins,
namely mix() (the boolean variant) and ldexp(). Note that with NIR and
SSA we'll start using them for more stuff, like optimizing away if
(...) { foo = 1; } else { foo = 2; } and flattening if's that are too
deep for the driver since conditional moves will be going away. The
expectation is that backends that prefer conditional moves will lower
the csel to a move and a conditional move, and fancier things like
psi-SSA can be used in the backend to turn the csel's into predicates
on the source instructions.

Connor

>
> Looks good in any case.
>
> Roland
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev