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

[Nicolai Hähnle]

On 29.09.2017 00:51, Matt Turner wrote:
> 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.

Splitting exp into two parts is a pretty cool trick, but to be honest I 
did test the new GLSL lowering quite extensively and don't see a good 
reason to go through all that again since we're not actually going to 
use it for long anymore.

In your earlier email, you asked what our hardware actually does. I 
can't speak for pre-GCN, which may have been more limited, but GCN has a 
complete implementation that covers all the cases. It even generates 
denormals :)

Cheers,
Nicolai


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


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.