JitArm64: Avoid double rounding in fctiwzx #9016
Conversation
FCVT doesn't necessarily round to zero, so the result might be inaccurate if we use it. To ensure correct rounding, we use FCVTS from double FPR to 32-bit GPR. Unfortunately, FCVTS can't do double FPR to single FPR.
|
(The reason we cannot use FCVT is not that it doesn't always round-towards-zero. The problem is it only converts between different floating-point formats but fctiwzx converts to integer format.) LGTM. Please test though. |
|
That is the reason why we can't use just FCVT, forgoing FCVTS. What I am arguing for is why we shouldn't use FVCT in combination with FCVTS. I have tested two games that break when intentionally messing up the emitted code for this instruction, and they seem to work fine. |
| ARM64Reg V1 = gpr.GetReg(); | ||
|
|
||
| m_float_emit.FCVTS(V1, EncodeRegToDouble(VB), ROUND_Z); | ||
| m_float_emit.FMOV(EncodeRegToSingle(VD), V1); |
There was a problem hiding this comment.
Why do you need a temp register here?
m_float_emit.FCVTS(EncodeRegToSingle(VD), EncodeRegToDouble(VB), ROUND_Z);
m_float_emit.FMOV(EncodeRegToSingle(VD), EncodeRegToSingle(VD));
There was a problem hiding this comment.
My understanding is that the FCVT*S instruction family supports:
- f64-to-s32 with the result in a general purpose register
- f64-to-s64 with the result in a SIMD&FP register
but we need f64-to-s32 and the result in a SIMD&FP register.
There was a problem hiding this comment.
Oh, I have not noticed that this temporary register was a GPR one, my bad.
FCVT doesn't necessarily round to zero, so the result might be inaccurate if we use it. To ensure correct rounding, we use FCVTS from double FPR to 32-bit GPR. Unfortunately, FCVTS can't do double FPR to single FPR.
This is my first time making a JitArm64 PR, so apologies if the change doesn't make sense :)