1. Patchew
  2. QEMU
  3. View series

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

Simon Scherer posted 1 patch 4 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260828082235.128360-1-scherer.simon89@gmail.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
target/i386/tcg/fpu_helper.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH v2] target/i386: clear C1 for all x87 compare instructions
Posted by Simon Scherer 4 weeks, 1 day 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, adding the clear
into the existing fpus mask for fcom/fucom since fcom_ccval never sets
bit 9. For fcomi/fucomi add a new separate clear.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4378
Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
---
v2: merge the C1 clear into the existing fpus mask for
    fcom_ST0_FT0/fucom_ST0_FT0 (Richard Henderson)

Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
---
 target/i386/tcg/fpu_helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index b812125efa..aa6527b0f5 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -531,7 +531,8 @@ void helper_fcom_ST0_FT0(CPUX86State *env)
     FloatRelation ret;
 
     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 = (env->fpus & ~0x4700) | fcom_ccval[ret + 1];
     merge_exception_flags(env, old_flags);
 }
 
@@ -541,7 +542,8 @@ void helper_fucom_ST0_FT0(CPUX86State *env)
     FloatRelation ret;
 
     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 = (env->fpus & ~0x4700) | fcom_ccval[ret + 1];
     merge_exception_flags(env, old_flags);
 }
 
@@ -556,6 +558,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 +572,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 v2] target/i386: clear C1 for all x87 compare instructions
Posted by Simon Scherer 3 weeks, 3 days ago 
Update on this patch: I tested the C1 behavior directly on real x86-64
hardware again for all of these instructions, and it turns out the
FCOMI/FCOMIP/FUCOMI/FUCOMIP part is wrong.

FCOM/FCOMP/FCOMPP/FUCOM/FUCOMP/FUCOMPP/FICOM/FICOMP do genuinely clear
C1 on real hardware, matching the SDM and the
helper_fcom_ST0_FT0/helper_fucom_ST0_FT0 fix in this patch.

FCOMI/FCOMIP/FUCOMI/FUCOMIP, however, leave C1 unchanged on real
silicon, despite the SDM listing "C1 Set to 0" under FPU Flags
Affected for these too. My best guess is that this line in the SDM was
carried over from the FCOM/FUCOM description.

I will send a v3 that drops the
helper_fcomi_ST0_FT0/helper_fucomi_ST0_FT0 changes.

Thanks,
Simon

On Fri, Aug 28, 2026 at 10:22 AM Simon Scherer
<scherer.simon89@gmail.com> wrote:
>
> 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, adding the clear
> into the existing fpus mask for fcom/fucom since fcom_ccval never sets
> bit 9. For fcomi/fucomi add a new separate clear.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4378
> Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
> ---
> v2: merge the C1 clear into the existing fpus mask for
>     fcom_ST0_FT0/fucom_ST0_FT0 (Richard Henderson)
>
> Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
> ---
>  target/i386/tcg/fpu_helper.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
> index b812125efa..aa6527b0f5 100644
> --- a/target/i386/tcg/fpu_helper.c
> +++ b/target/i386/tcg/fpu_helper.c
> @@ -531,7 +531,8 @@ void helper_fcom_ST0_FT0(CPUX86State *env)
>      FloatRelation ret;
>
>      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 = (env->fpus & ~0x4700) | fcom_ccval[ret + 1];
>      merge_exception_flags(env, old_flags);
>  }
>
> @@ -541,7 +542,8 @@ void helper_fucom_ST0_FT0(CPUX86State *env)
>      FloatRelation ret;
>
>      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 = (env->fpus & ~0x4700) | fcom_ccval[ret + 1];
>      merge_exception_flags(env, old_flags);
>  }
>
> @@ -556,6 +558,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 +572,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
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.