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

[Matt Turner]

On Thu, Sep 28, 2017 at 3:42 PM, Matt Turner <mattst88 at gmail.com> wrote:
> 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.

Actually, it looks like the lowering Jason added to NIR is slightly
more complex than x * 2^exp, but still significantly simpler than the
madness of the GLSL IR lowering pass. It's probably too much to ask
since you didn't need to touch that lowering pass to solve your
problem to begin with, but it might be nice to just replace the entire
thing with what Jason did in nir_opt_algebraic.py.

I'm certainly not going to force you to do that.