[Mesa-dev] [PATCH 00/11] glsl, tgsi, radeonsi: ldexp and frexp bug fixes and features

[Matt Turner]

On Sat, Sep 16, 2017 at 4:23 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> Hi all,
>
> This series was motivated by radeonsi failing some ldexp tests due to
> not handling denorms correctly and not handling overflows (which GLSL
> doesn't require, but GLSL ES does).
>
> The first patch fixes the GLSL IR lowering of ldexp() to handle all cases
> fully except:
>
> 1. Denorms; they're annoying and therefore all flushed to zero.
>
> 2. Exponent 32-bit overflow. This would be easy to fix at the cost of
>    an additional min() instruction, but neither GLSL nor GLSL ES
>    requires it.

Heh, this brings back memories (and pain) of
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=11180

In fact, I think comment 17 notes those two "bugs" exactly. Unless
something's changed, (2) should be undefined behavior.

When I implemented the lowering pass, I argued that ldexp(x, exp)
should not be directly implementable as x * 2^exp -- because why are
we adding a built-in function for such a trivial function?!

Consider cases like

ldexp(9.17219e+025, -154) = 4.01655e-021 // final exp = 86 - 154 = -68
ldexp(2.53619e-030, 146) = 2.26236e+014 // final exp = -99 + 146 = 47

The exp parameter is outside of the normal range of 32-bit
floating-point exponents, but there's clearly an in-range result.

Anyway, Khronos decided to go with allowing it to be implemented as x
* 2^exp, and when we switched the i965 driver to NIR stopped using the
GLSL lowering of ldexp.

I appreciate you fixing up the lowering pass. We may just want to
replace it with a lowering to the trivial implementation, since that
seems to be what everyone else does.

In that bug, an AMD employee argued against making the requirements
"more strict", as he saw them. I tried to have him confirm what his
hardware's ldexp instruction did, but he wasn't interested. Maybe you
can confirm -- does your ldexp instruction handle cases like the
above?