helper_fcomi_ST0_FT0() and helper_fucomi_ST0_FT0() only cleared
CC_Z, CC_P, and CC_C before merging in the comparison result,
leaving CC_O, CC_S, and CC_A untouched from whatever they were
set to beforehand.

The Intel SDM documents FCOMI/FCOMIP/FUCOMI/FUCOMIP as setting OF,
SF, and AF to 0 unconditionally. The AMD manual doesn't mention them
at all. However, testing on multiple real Intel and AMD systems confirms
all three are unconditionally cleared regardless of the comparison
result or their prior value.

Since fcomi_ccval[] only ever contains CC_C, CC_Z, 0, or CC_Z|CC_P|CC_C,
and CC_O|CC_S|CC_Z|CC_A|CC_P|CC_C already covers every flag bit, CC_SRC
can be assigned from fcomi_ccval[ret + 1] directly instead of ORing it
into a masked cpu_cc_compute_all() result.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4133
Signed-off-by: Simon Scherer <scherer.simon89@gmail.com>
---
v2: Simplify per Richard Henderson's review: CC_O|CC_S|CC_A|CC_Z|CC_P|CC_C already covers
    every flag, so drop the now-pointless cpu_cc_compute_all() call and assign
    CC_SRC from fcomi_ccval[] directly.

 target/i386/tcg/fpu_helper.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 978eb1411b..b812125efa 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -550,12 +550,11 @@ static const int fcomi_ccval[4] = {CC_C, CC_Z, 0, CC_Z | CC_P | CC_C};
 void helper_fcomi_ST0_FT0(CPUX86State *env)
 {
     int old_flags = save_exception_flags(env);
-    int eflags;
     FloatRelation ret;
 
     ret = floatx80_compare(ST0, FT0, &env->fp_status);
-    eflags = cpu_cc_compute_all(env) & ~(CC_Z | CC_P | CC_C);
-    CC_SRC = eflags | fcomi_ccval[ret + 1];
+    /* OF, SF, and AF are unconditionally cleared to 0 */
+    CC_SRC = fcomi_ccval[ret + 1];
     CC_OP = CC_OP_EFLAGS;
     merge_exception_flags(env, old_flags);
 }
@@ -563,12 +562,11 @@ void helper_fcomi_ST0_FT0(CPUX86State *env)
 void helper_fucomi_ST0_FT0(CPUX86State *env)
 {
     int old_flags = save_exception_flags(env);
-    int eflags;
     FloatRelation ret;
 
     ret = floatx80_compare_quiet(ST0, FT0, &env->fp_status);
-    eflags = cpu_cc_compute_all(env) & ~(CC_Z | CC_P | CC_C);
-    CC_SRC = eflags | fcomi_ccval[ret + 1];
+    /* OF, SF, and AF are unconditionally cleared to 0 */
+    CC_SRC = fcomi_ccval[ret + 1];
     CC_OP = CC_OP_EFLAGS;
     merge_exception_flags(env, old_flags);
 }
-- 
2.53.0

On 8/6/26 23:28, Simon Scherer wrote:
> helper_fcomi_ST0_FT0() and helper_fucomi_ST0_FT0() only cleared
> CC_Z, CC_P, and CC_C before merging in the comparison result,
> leaving CC_O, CC_S, and CC_A untouched from whatever they were
> set to beforehand.
> 
> The Intel SDM documents FCOMI/FCOMIP/FUCOMI/FUCOMIP as setting OF,
> SF, and AF to 0 unconditionally. The AMD manual doesn't mention them
> at all. However, testing on multiple real Intel and AMD systems confirms
> all three are unconditionally cleared regardless of the comparison
> result or their prior value.
> 
> Since fcomi_ccval[] only ever contains CC_C, CC_Z, 0, or CC_Z|CC_P|CC_C,
> and CC_O|CC_S|CC_Z|CC_A|CC_P|CC_C already covers every flag bit, CC_SRC
> can be assigned from fcomi_ccval[ret + 1] directly instead of ORing it
> into a masked cpu_cc_compute_all() result.
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/work_items/4133
> Signed-off-by: Simon Scherer<scherer.simon89@gmail.com>
> ---
> v2: Simplify per Richard Henderson's review: CC_O|CC_S|CC_A|CC_Z|CC_P|CC_C already covers
>      every flag, so drop the now-pointless cpu_cc_compute_all() call and assign
>      CC_SRC from fcomi_ccval[] directly.
> 
>   target/i386/tcg/fpu_helper.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Queued, thanks.

Paolo