[Mesa-dev] [PATCH] radeonsi: compute the absolute value before RSQ

[Roland Scheidegger]

Meh, I'm not really a big fan of such hacks. GPUs have support for NaNs
since ages, and while glsl is lenient the point stands that returning a
NaN is a more correct result, so doing extra work to get a wrong result
doesn't look all that great to me.
FWIW dx10 requires NaNs as results (for both sqrt and rsq). Maybe app
specific quirks (if said apps can't be fixed) would be better...
But well, it's your driver, so whatever floats your boat.

Roland


Am 05.01.2017 um 18:37 schrieb Marek Olšák:
> Shouldn't we also use abs for SQRT? For example, this adds abs for
> both RSQ and SQRT:
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__cgit.freedesktop.org_-7Emareko_mesa_commit_-3Fid-3D5e0fb661a8e6ac5f7b2245dd31595155128e0664&d=DgIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wUYi2DhIZXHwD19wGIoEgceIF0cE0chTe7swjZ4MLVs&s=ZqrwtvXGdvXw-YJQtVnWo7yBeH5lhM3jtnYaa6vFZJY&e= 
> 
> Marek
> 
> On Thu, Jan 5, 2017 at 5:47 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> As explained by Nicolai, it seems like D3D always compute the
>> absolute value while GLSL says that the result of inversesqrt()
>> is undefined if x <= 0. Using the absolute value looks like safer
>> especially when the game has been ported from D3D to GL.
>>
>> This gets rid of the NaN values in the "Spec Ops: The Line" game
>> as well as the black squares.
>>
>> Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D97338&d=DgIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wUYi2DhIZXHwD19wGIoEgceIF0cE0chTe7swjZ4MLVs&s=gjxDxmhEtQDYO34rucXJ6nIDojabOhpTAx1rNUP6X-g&e= 
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>
>> Nouveau also computes the absolute value before emitting RSQ.
>>
>>  src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>> index 1966752cc0..ec6d6b0534 100644
>> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>> @@ -754,9 +754,12 @@ static void emit_rsq(const struct lp_build_tgsi_action *action,
>>                      struct lp_build_tgsi_context *bld_base,
>>                      struct lp_build_emit_data *emit_data)
>>  {
>> -       LLVMValueRef sqrt =
>> -               lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_SQRT,
>> -                                        emit_data->args[0]);
>> +       LLVMValueRef abs, sqrt;
>> +
>> +       abs = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_ABS,
>> +                                      emit_data->args[0]);
>> +
>> +       sqrt = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_SQRT, abs);
>>
>>         emit_data->output[emit_data->chan] =
>>                 lp_build_emit_llvm_binary(bld_base, TGSI_OPCODE_DIV,
>> --
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DgIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wUYi2DhIZXHwD19wGIoEgceIF0cE0chTe7swjZ4MLVs&s=boy8sEj2W3FP0LnjHsSnUxWrR98aM22Qq-uxvFIqMlQ&e= 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=DgIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=_QIjpv-UJ77xEQY8fIYoQtr5qv8wKrPJc7v7_-CYAb0&m=wUYi2DhIZXHwD19wGIoEgceIF0cE0chTe7swjZ4MLVs&s=boy8sEj2W3FP0LnjHsSnUxWrR98aM22Qq-uxvFIqMlQ&e= 
>