[v1] target/i386: clear C1 for all x87 compare instructions

[PATCH] target/i386: clear C1 for all x87 compare instructions

Posted by Simon Scherer 1 month ago

The SDM specifies that FCOM/FCOMP/FCOMPP/FUCOM/FUCOMP/FUCOMPP/FICOM/FICOMP/
FCOMI/FCOMIP/FUCOMI/FUCOMIP unconditionally clear C1 in the FPU status word,
regardless of the comparison result.

helper_fcom_ST0_FT0/helper_fucom_ST0_FT0 only cleared C3, C2, C0 (mask
0x4500) before OR-ing in the comparison result, leaving C1 (bit 9) at
whatever value it already had. FICOM/FICOMP dispatch through the same
helpers after converting their integer operand, so they inherited the
same bug.

helper_fcomi_ST0_FT0/helper_fucomi_ST0_FT0 never touched the FPU status
word at all, so C1 was left untouched by those too.

This patch clears C1 explicitly in all four helpers.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4378
Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
---
 target/i386/tcg/fpu_helper.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index b812125efa..49459b5bc1 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -532,6 +532,8 @@ void helper_fcom_ST0_FT0(CPUX86State *env)
 
     ret = floatx80_compare(ST0, FT0, &env->fp_status);
     env->fpus = (env->fpus & ~0x4500) | fcom_ccval[ret + 1];
+    /* C1 is unconditionally cleared to 0 */
+    env->fpus &= ~0x0200;
     merge_exception_flags(env, old_flags);
 }
 
@@ -542,6 +544,8 @@ void helper_fucom_ST0_FT0(CPUX86State *env)
 
     ret = floatx80_compare_quiet(ST0, FT0, &env->fp_status);
     env->fpus = (env->fpus & ~0x4500) | fcom_ccval[ret + 1];
+    /* C1 is unconditionally cleared to 0 */
+    env->fpus &= ~0x0200;
     merge_exception_flags(env, old_flags);
 }
 
@@ -556,6 +560,8 @@ void helper_fcomi_ST0_FT0(CPUX86State *env)
     /* OF, SF, and AF are unconditionally cleared to 0 */
     CC_SRC = fcomi_ccval[ret + 1];
     CC_OP = CC_OP_EFLAGS;
+    /* C1 is unconditionally cleared to 0 */
+    env->fpus &= ~0x0200;
     merge_exception_flags(env, old_flags);
 }
 
@@ -568,6 +574,8 @@ void helper_fucomi_ST0_FT0(CPUX86State *env)
     /* OF, SF, and AF are unconditionally cleared to 0 */
     CC_SRC = fcomi_ccval[ret + 1];
     CC_OP = CC_OP_EFLAGS;
+    /* C1 is unconditionally cleared to 0 */
+    env->fpus &= ~0x0200;
     merge_exception_flags(env, old_flags);
 }
 
-- 
2.53.0

Re: [PATCH] target/i386: clear C1 for all x87 compare instructions

Posted by Richard Henderson 1 month ago

On 8/27/26 08:26, Simon Scherer wrote:
>       env->fpus = (env->fpus & ~0x4500) | fcom_ccval[ret + 1];
> +    /* C1 is unconditionally cleared to 0 */
> +    env->fpus &= ~0x0200;

Then merge the mask with the previous: ~0x4700.

r~

Re: [PATCH] target/i386: clear C1 for all x87 compare instructions

Posted by Simon Scherer 4 weeks, 1 day ago

Good point, sent a v2.

On Thu, Aug 27, 2026 at 7:41 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/27/26 08:26, Simon Scherer wrote:
> >       env->fpus = (env->fpus & ~0x4500) | fcom_ccval[ret + 1];
> > +    /* C1 is unconditionally cleared to 0 */
> > +    env->fpus &= ~0x0200;
>
> Then merge the mask with the previous: ~0x4700.
>
> r~