On 12/11/20 2:31 AM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana <[email protected]>
> ---
> target/i386/cpu.h | 97 ++---------------------------
> target/i386/tcg/helper-tcg.h | 112 ++++++++++++++++++++++++++++++++++
> target/i386/tcg/tcg-cpu.h | 15 +++++
> target/i386/cpu.c | 33 ++++------
> target/i386/helper.c | 23 -------
> target/i386/tcg/bpt_helper.c | 1 +
> target/i386/tcg/cc_helper.c | 1 +
> target/i386/tcg/excp_helper.c | 1 +
> target/i386/tcg/fpu_helper.c | 33 +++++-----
> target/i386/tcg/int_helper.c | 1 +
> target/i386/tcg/mem_helper.c | 1 +
> target/i386/tcg/misc_helper.c | 1 +
> target/i386/tcg/mpx_helper.c | 1 +
> target/i386/tcg/seg_helper.c | 1 +
> target/i386/tcg/smm_helper.c | 2 +
> target/i386/tcg/svm_helper.c | 1 +
> target/i386/tcg/tcg-cpu.c | 71 +++++++++++++++++++++
> target/i386/tcg/translate.c | 1 +
> target/i386/tcg/meson.build | 1 +
> 19 files changed, 244 insertions(+), 153 deletions(-)
> create mode 100644 target/i386/tcg/helper-tcg.h
> create mode 100644 target/i386/tcg/tcg-cpu.h
> create mode 100644 target/i386/tcg/tcg-cpu.c
This is doing a lot more than $SUBJECT, which afaict would just be the creation
of target/i386/tcg/tcg-cpu.c.
> +#define CC_DST (env->cc_dst)
> +#define CC_SRC (env->cc_src)
> +#define CC_SRC2 (env->cc_src2)
> +#define CC_OP (env->cc_op)
Why are these moving within cpu.h? If they move at all, they should be moved
out of here. Better if they're simply removed -- this is a silly wrapping of
"env->foo" from ancient days.
> +/* float macros */
> +#define FT0 (env->ft0)
> +#define ST0 (env->fpregs[env->fpstt].d)
> +#define ST(n) (env->fpregs[(env->fpstt + (n)) & 7].d)
> +#define ST1 ST(1)
Any chance these can move to fpu_helper.c? Or, in the case of FT0, be
eliminated?
> +/* cc_helper.c */
> +extern const uint8_t parity_table[256];
We should probably remove this and just use ctpop(x) & 1.
> +/*
> + * NOTE: the translator must set DisasContext.cc_op to CC_OP_EFLAGS
> + * after generating a call to a helper that uses this.
> + */
> +static inline void cpu_load_eflags(CPUX86State *env, int eflags,
> + int update_mask)
> +{
> + CC_SRC = eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
> + CC_OP = CC_OP_EFLAGS;
> + env->df = 1 - (2 * ((eflags >> 10) & 1));
> + env->eflags = (env->eflags & ~update_mask) |
> + (eflags & update_mask) | 0x2;
> +}
This is complex enough I would be in favor of moving it out-of-line somewhere
convenient.
> +++ b/target/i386/tcg/tcg-cpu.h
> @@ -0,0 +1,15 @@
> +/*
> + * i386 TCG CPU class initialization
> + *
> + * Copyright 2020 SUSE LLC
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef TCG_CPU_H
> +#define TCG_CPU_H
> +
> +void tcg_cpu_common_class_init(CPUClass *cc);
> +
Why does the new file need it's own header, with one declaration?
r~