[Mesa-dev] [PATCH] nir/spirv/glsl450: rewrite atan2 to deal with denorms / infinities

[Francisco Jerez]

"Juan A. Suarez Romero" <jasuarez at igalia.com> writes:

> On Tue, 2017-01-17 at 11:34 +0100, Juan A. Suarez Romero wrote:
>> > The above does not necessarily sum to "we shouldn't fix it" but it probably does mean it's low-priority at best and we need to be careful.
>> > 
>> > Looking a bit into the math of atan2, I'm not convinced flushing denorms to zero is actually what we want to do.  For x, I think it's valid, but as y approaches 0, you get +/- pi/2 depending on whether y positive or negative.  As both approach zero, it's undefined.  As you approach infinity, it approaches something in the interval [-pi/2, pi/2] but it depends on the direction of approach.  If you do "man atan2" you'll see this all layed out in a horrifying amount of detail (somewhere around a dozen different cases).
>> > 
>> > 
>> 
>> Yes, originally I only flushed 'x'. But then also flushed 'y' to keep coherence. But didn't think about that corner case you mention. I think we can remove the flush to 0 in 'y'.
>
> Actually, after doing  a couple of tests more, I don't think we need to flush 'x' any more.
>
> The thing is that originally I added the flush to 0 to solve a couple of cases from Vulkan CTS atan2 tests. But later I modified the code to cope with infinities, which lead a refactoring of code. I think this refactoring made the flush to 0 not required any more.
>
> I've run the tests again getting rid of both flushes, and seems results are exactly the same as with the flush. So maybe they are not required after all.
>

So what exactly from the many checks you added makes the tests pass?
IOW, what property do the vulkan CTS tests take for granted that the
current atan2 implementation doesn't fulfill?

>
> 	J.A.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170117/b707aac2/attachment.sig>