On Sat, Sep 16, 2017 at 4:23 AM, Nicolai Hähnle <nhaeh...@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? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
