Re: [PATCH] target/i386: fix ADOX followed by ADCX

[Richard Henderson <richard.henderson@linaro.org>]

On 1/30/23 22:54, Paolo Bonzini wrote:
> When ADCX is followed by ADOX or vice versa, the second instruction's
> carry comes from EFLAGS.  This is handled by this bit of gen_ADCOX:
> 
>          tcg_gen_extract_tl(carry_in, cpu_cc_src,
>              ctz32(cc_op == CC_OP_ADCX ? CC_C : CC_O), 1);
> 
> Unfortunately, in this case cc_op has been overwritten by the previous
> "if" statement to CC_OP_ADCOX.  This works by chance when the first
> instruction is ADCX; however, if the first instruction is ADOX,
> ADCX will incorrectly take its carry from OF instead of CF.
> 
> Fix by moving the computation of the new cc_op at the end of the function.
> The included exhaustive test case fails without this patch and passes
> afterwards.
> 
> Because ADCX/ADOX need not be invoked through the VEX prefix, this
> regression bisects to commit 16fc5726a6e2 ("target/i386: reimplement
> 0x0f 0x38, add AVX", 2022-10-18).  However, the mistake happened a
> little earlier, when BMI instructions were rewritten using the new
> decoder framework.
> 
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1471
> Reported-by: Paul Jolly<https://gitlab.com/myitcv>
> Fixes: 1d0b926150e5 ("target/i386: move scalar 0F 38 and 0F 3A instruction to new decoder", 2022-10-18)
> Cc:qemu-stable@nongnu.org
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   target/i386/tcg/emit.c.inc       | 20 +++++----
>   tests/tcg/i386/Makefile.target   |  6 ++-
>   tests/tcg/i386/test-i386-adcox.c | 75 ++++++++++++++++++++++++++++++++
>   3 files changed, 91 insertions(+), 10 deletions(-)
>   create mode 100644 tests/tcg/i386/test-i386-adcox.c

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

r~