Re: [Qemu-devel] [RFC PATCH v2 23/39] target/i386: introduce instruction translator macros

[Jan Bobek]

On 8/13/19 2:30 AM, Richard Henderson wrote:
> On 8/10/19 5:12 AM, Jan Bobek wrote:
>> +#define CASES_LEG_NP_0F_W0(opcode)  \
>> +case opcode | M_0F | W_0:
>> +#define CASES_LEG_NP_0F_W1(opcode)  \
>> +case opcode | M_0F | W_1:
>> +#define CASES_LEG_F3_0F_W0(opcode)  \
>> +case opcode | M_0F | P_F3 | W_0:
>> +#define CASES_LEG_F3_0F_W1(opcode)  \
>> +case opcode | M_0F | P_F3 | W_1:
>> +
>> +#define LEG(p, m, w)\
>> +CASES_LEG_ ## p ## _ ## m ## _W ## w
>> +#define INSN(mnem, cases, opcode, feat) \
>> +cases(opcode)   \
> 
> It appears as if you don't need the CASES_* macros here.
> 
> #define LEG(p, m, w, op) \
>case P_##p | M_##m | W_##2 | op
> 
> #define INSN(mnem, leg, feat) \
>leg: translate_insn(env, s, CK_CPUID_##feat, gen_insn(mnem));
> 
> so long as P_NP is in the enumeration above with value 0.
> 
> Unless there's some other reason that opcode needs to stay separate?

I was thinking ahead with the CASES_* macros here: if I have LIG
and/or WIG in the VEX prefix, I'll need more than one case label,
but only one label in other cases. However, that's not a reason
for the opcode to be separate, and I think I like it stashed with
the rest of the prefix fields better.

-Jan
 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH v2 23/39] target/i386: introduce instruction translator macros

[Richard Henderson]

On 8/10/19 5:12 AM, Jan Bobek wrote:
> +#define CASES_LEG_NP_0F_W0(opcode)  \
> +case opcode | M_0F | W_0:
> +#define CASES_LEG_NP_0F_W1(opcode)  \
> +case opcode | M_0F | W_1:
> +#define CASES_LEG_F3_0F_W0(opcode)  \
> +case opcode | M_0F | P_F3 | W_0:
> +#define CASES_LEG_F3_0F_W1(opcode)  \
> +case opcode | M_0F | P_F3 | W_1:
> +
> +#define LEG(p, m, w)\
> +CASES_LEG_ ## p ## _ ## m ## _W ## w
> +#define INSN(mnem, cases, opcode, feat) \
> +cases(opcode)   \

It appears as if you don't need the CASES_* macros here.

#define LEG(p, m, w, op) \
   case P_##p | M_##m | W_##2 | op

#define INSN(mnem, leg, feat) \
   leg: translate_insn(env, s, CK_CPUID_##feat, gen_insn(mnem));

so long as P_NP is in the enumeration above with value 0.

Unless there's some other reason that opcode needs to stay separate?


r~

[Qemu-devel] [RFC PATCH v2 23/39] target/i386: introduce instruction translator macros

[Jan Bobek]

Instruction "translators" are responsible for decoding and loading
instruction operands, calling the passed-in code generator, and
storing the operands back (if applicable). Once a translator returns,
the instruction has been translated to TCG ops, hence the name.

Signed-off-by: Jan Bobek 
---
 target/i386/translate.c | 288 
 1 file changed, 288 insertions(+)

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 30180d1c25..0da064d5fd 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -4715,6 +4715,222 @@ static int ck_cpuid(CPUX86State *env, DisasContext *s, 
int ck_cpuid_feat)
 #define gen_insn_wrrr(mnem, opW1, opR1, opR2, opR3) \
 gen_ ## mnem ## _ ## opW1 ## opR1 ## opR2 ## opR3
 
+/*
+ * Instruction translators
+ */
+#define translate_insn_r(opR1)  \
+translate_insn_r_ ## opR1
+#define translate_insn_rr(opR1, opR2)   \
+translate_insn_rr_ ## opR1 ## opR2
+#define translate_insn_w(opW1)  \
+translate_insn_w_ ## opW1
+#define translate_insn_wr(opW1, opR1)   \
+translate_insn_wr_ ## opW1 ## opR1
+#define translate_insn_wrr(opW1, opR1, opR2)\
+translate_insn_wrr_ ## opW1 ## opR1 ## opR2
+#define translate_insn_wrrr(opW1, opR1, opR2, opR3) \
+translate_insn_wrrr_ ## opW1 ## opR1 ## opR2 ## opR3
+#define translate_group(grpname)\
+translate_group_ ## grpname
+
+static void translate_insn(
+CPUX86State *env, DisasContext *s, int ck_cpuid_feat,
+void (*gen_insn_fp)(CPUX86State *, DisasContext *))
+{
+if (ck_cpuid(env, s, ck_cpuid_feat)) {
+gen_illegal_opcode(s);
+return;
+}
+
+(*gen_insn_fp)(env, s);
+}
+
+#define TRANSLATE_INSN_R(opR1)  \
+static void translate_insn_r(opR1)( \
+CPUX86State *env, DisasContext *s, int modrm, int ck_cpuid_feat, \
+void (*gen_insn_fp)(CPUX86State *, DisasContext *, insnop_t(opR1))) \
+{   \
+insnop_t(opR1) arg1;\
+\
+if (ck_cpuid(env, s, ck_cpuid_feat) \
+|| insnop_init(opR1)(env, s, modrm, &arg1)) {   \
+gen_illegal_opcode(s);  \
+return; \
+}   \
+\
+insnop_prepare(opR1)(env, s, modrm, &arg1); \
+(*gen_insn_fp)(env, s, arg1);   \
+}
+
+#define TRANSLATE_INSN_RR(opR1, opR2)   \
+static void translate_insn_rr(opR1, opR2)(  \
+CPUX86State *env, DisasContext *s, int modrm, int ck_cpuid_feat, \
+void (*gen_insn_fp)(CPUX86State *, DisasContext *, insnop_t(opR1), \
+insnop_t(opR2)))\
+{   \
+insnop_t(opR1) arg1;\
+insnop_t(opR2) arg2;\
+\
+if (ck_cpuid(env, s, ck_cpuid_feat) \
+|| insnop_init(opR1)(env, s, modrm, &arg1)  \
+|| insnop_init(opR2)(env, s, modrm, &arg2)) {   \
+gen_illegal_opcode(s);  \
+return; \
+}   \
+\
+insnop_prepare(opR1)(env, s, modrm, &arg1); \
+insnop_prepare(opR2)(env, s, modrm, &arg2); \
+(*gen_insn_fp)(env, s, arg1, arg2); \
+}
+
+#define TRANSLATE_INSN_W(opW1)  \
+static void translate_insn_w(opW1)( \
+CPUX86State *env, DisasContext *s, int modrm, int ck_cpuid_feat, \
+void (*gen_insn_fp)(CPUX86State *, DisasContext *, insnop_t(opW1))) \
+{   \
+insnop_t(opW1) ret; \
+\
+if (ck_cpuid(env, s, ck_cpuid_feat) \
+|| insnop_init(opW1)(env, s, modrm, &ret)) {