[Mesa-dev] [PATCH] compiler/glsl: fix precision problem of tanh

[Jason Ekstrand]

On Thu, Dec 8, 2016 at 5:03 PM, Haixia Shi <hshi at chromium.org> wrote:

> Clamp input scalar value to range [-10, +10] to avoid precision problems
> when the absolute value of input is too large.
>
> Fixes dEQP-GLES3.functional.shaders.builtin_functions.precision.tanh.*
> test
> failures.
>
> Signed-off-by: Haixia Shi <hshi at chromium.org>
> Cc: Jason Ekstrand <jason at jlekstrand.net>,
> Cc: Stéphane Marchesin <marcheu at chromium.org>,
> Cc: Kenneth Graunke <kenneth at whitecape.org>
>
> Change-Id: I324c948b3323ff8107127c42934f14459e124b95
> ---
>  src/compiler/glsl/builtin_functions.cpp | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/glsl/builtin_functions.cpp
> b/src/compiler/glsl/builtin_functions.cpp
> index 3e4bcbb..4ede925 100644
> --- a/src/compiler/glsl/builtin_functions.cpp
> +++ b/src/compiler/glsl/builtin_functions.cpp
> @@ -3563,9 +3563,13 @@ builtin_builder::_tanh(const glsl_type *type)
>     ir_variable *x = in_var(type, "x");
>     MAKE_SIG(type, v130, 1, x);
>
> +   /* clamp x to [-10, +10] to avoid precision problems */
>

Would you mind adding something like this to your comment:

When x > 10, e^-x is so small relative to e^x that it gets flushed to zero
in the computation e^x + e^-x.  The same happens in the other direction
when x < -10.

With that added,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

Chatting with Ken a bit, I realized why you used 20 before.  That's the
threshold needed if you're doing the computation in double-precision.
However, GLSL doesn't define a double form of tanh so that's not really a
problem.


> +   ir_variable *t = body.make_temp(type, "tmp");
> +   body.emit(assign(t, min2(max2(x, imm(-10.0f)), imm(10.0f))));
> +
>     /* (e^x - e^(-x)) / (e^x + e^(-x)) */
> -   body.emit(ret(div(sub(exp(x), exp(neg(x))),
> -                     add(exp(x), exp(neg(x))))));
> +   body.emit(ret(div(sub(exp(t), exp(neg(t))),
> +                     add(exp(t), exp(neg(t))))));
>
>     return sig;
>  }
> --
> 2.8.0.rc3.226.g39d4020
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161208/0e82c860/attachment-0001.html>