Re: [PATCH 02/25] target/i386: rewrite flags writeback for ADCX/ADOX

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

On 6/8/24 01:40, Paolo Bonzini wrote:
> Avoid using set_cc_op() in preparation for implementing APX; treat
> CC_OP_EFLAGS similar to the case where we have the "opposite" cc_op
> (CC_OP_ADOX for ADCX and CC_OP_ADCX for ADOX), except the resulting
> cc_op is not CC_OP_ADCOX. This is written easily as two "if"s, whose
> conditions are both false for CC_OP_EFLAGS, both true for CC_OP_ADCOX,
> and one each true for CC_OP_ADCX/ADOX.
> 
> The new logic also makes it easy to drop usage of tmp0.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/cpu.h          |  9 +++---
>   target/i386/tcg/emit.c.inc | 61 ++++++++++++++++++++++----------------
>   2 files changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 8fe28b67e0f..ee873a0ed84 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1260,6 +1260,8 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
>   /* Use a clearer name for this.  */
>   #define CPU_INTERRUPT_INIT      CPU_INTERRUPT_RESET
>   
> +#define CC_OP_HAS_EFLAGS(op) ((op) >= CC_OP_EFLAGS && (op) <= CC_OP_ADCOX)
> +
>   /* Instead of computing the condition codes after each x86 instruction,
>    * QEMU just stores one operand (called CC_SRC), the result
>    * (called CC_DST) and the type of operation (called CC_OP). When the
> @@ -1270,6 +1272,9 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
>   typedef enum {
>       CC_OP_DYNAMIC, /* must use dynamic code to get cc_op */
>       CC_OP_EFLAGS,  /* all cc are explicitly computed, CC_SRC = flags */
> +    CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest.  */
> +    CC_OP_ADOX, /* CC_DST = O, CC_SRC = rest.  */
> +    CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest.  */

Comment for ADOX is (and was) wrong -- SRC2 = O.
That made review a bit confusing... :-)

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


r~