JIT: fix PPC_FP snan/qnan handling

[FioraAeterna]

Needs testing to see if this fixes Beyond Good and Evil

[comex]

Might it be possible to merge those branches into one somehow?

[FioraAeterna]

I can't think of any way without adding even more uops

[FioraAeterna]

squees all over the PR

IT FIXES IT

[PatrickFerry]

You're a wizard. Good job, how did you manage to fix it without having the game?
squee

[FioraAeterna]

I figured that the only way things could be broken is if my conversion algorithm was actually lossy on some inputs when it shouldn't be, so I wrote a C implementation and checked it on all possible inputs (2^32 possible float bit patterns)... and found I was right.

Then had to fix it <___<;

[comex]

Just to recap what I said on IRC yesterday, I'd appreciate if you checked before merging whether using a cmov or whatever to avoid two branches can reduce the performance hit. Thanks...

[FioraAeterna]

I don't know any good way to do this with a cmov that won't balloon the instruction count, do you have any suggestions?

[comex]

Something like xor %ecx, %ecx; cmovz %ecx, %eax? After the subtraction, so it would become zero, which <= 3.

Okay, I don't know if this would be a good idea or not. But it saves a useless branch on every operation, and is one less byte than a long jne...

[JMC47]

This has little to no performance drop for me. It fixes some errors that probably need to be fixed as well.

[FioraAeterna]

8 seconds on povray is at least pretty significant to me, though certainly not that bad. I really do welcome suggestions on specific code to improve any of the asm paths here though, since this is very much "hyperoptimize a small instruction sequence that will be in a million places"

[magumagu]

For ConvertDoubleToSingle, you could just fast-path exponents in the range 0x387 < e < 0x7f8, or something like that; that should be the same cost as the old code on the fast path, although it throws more inputs onto the slow path. I guess the primary issue with this is that you want 0.0 on the fast path... but it might be good enough special-case it on the slow path? Might depend on what the inputs look like.

[Tilka]

How about this "high-level" approach for ConvertDoubleToSingle()'s fast path?

static const __m128i GC_ALIGNED16(not_sign_bit) = _mm_set_epi64x(0xffffffffffffffff, 0x7fffffffffffffff);
static const double GC_ALIGNED16(min_norm_single) = std::numeric_limits<float>::min();

UCOMISD(src, R(src));
FixupBranch nanConversion = J_CC(CC_P, true);
VPAND(XMM0, src, M(&not_sign_bit));
UCOMISD(XMM0, M(&min_norm_single));
FixupBranch denormalConversion = J_CC(CC_B, true);
CVTSD2SS(dst, R(src));
// ...

C++ equivalent would be:

if (src != src) {
  // nanConversion
} else if (std::abs(src) < std::numeric_limits<float>::min()) {
  // denormalConversion (including zero :/ )
} else {
  // just CVTSD2SS
}

In the four POV-Ray runs I did, my code was 1-2 seconds slower. I do think it's a lot more readable though. Thoughts?

[Tilka]

I'm OK with this. And by "OK" I mean "aww yeah".

[FioraAeterna]

Much better now; thanks flacs!

[OussamaDanba]

LGTM. This fixes Beyond Good and Evil (issue 7897) and performance is nearly identical.

[skidau]

Code looks fine to me.

[psennermann]

Just wanted to report that in fact performance isn't really so identical: on my system this commit causes a speed loss around 3%

[OussamaDanba]

A 3% speed loss isn't actually that big compared to one of the first version which had me drop almost twice as much as that. AFAIK @FioraAeterna was aware that there would be a performance loss and that it would be inevitable.

[psennermann]

I wanted only to report/prove that the statement "performance is nearly identical" was definitely wrong ;)

[FioraAeterna]

I was going off someone else's benchmarks, sorry!

[JMC47]

Speed loss is fine once this actually works. PPC_FP stuff is things we kind of need.