On 4/25/22 00:01, Paul Brook wrote:
+/* If a VEX prefix is used then it must have V=1111b */
+#define CHECK_AVX_V0(s) do { \
+ CHECK_AVX(s); \
+ if ((s->prefix & PREFIX_VEX) && (s->vex_v != 0)) \
+ goto illegal_op; \
+ } while (0)
+
What do you think about
#define CHECK_AVX(s, flags) \
do {
if ((s->prefix & PREFIX_VEX) && !(env->hflags & HF_AVX_EN_MASK)) {
goto illegal_op;
}
if ((flags) & SSE_OPF_AVX2) {
CHECK_AVX2(s);
}
if ((flags) & SSE_OPF_AVX_128) {
CHECK_AVX_128(s);
}
if ((flags) & SSE_OPF_V0) {
CHECK_V0(s);
}
}
Macros such as CHECK_AVX_V0_128(s) would become CHECK_AVX(s, SSE_OPF_V0
| SSE_OPF_AVX_128); a bit longer but still bearable. And here you would
have:
case 0x210: /* movss xmm, ea */
if (mod != 3) {
+ CHECK_AVX_V0_128(s);
gen_lea_modrm(env, s, modrm);
gen_op_ld_v(s, MO_32, s->T0, s->A0);
tcg_gen_st32_tl(s->T0, cpu_env,
@@ -3379,6 +3432,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s,
int b,
tcg_gen_st32_tl(s->T0, cpu_env,
offsetof(CPUX86State,
xmm_regs[reg].ZMM_L(3)));
} else {
+ CHECK_AVX_128(s);
CHECK_AVX(s, SSE_OPF_AVX_128);
if (mod != 3) {
CHECK_V0(s);
...
} else {
...
}
Another possibility is to add SSE_OPF_V0_MEM (i.e. V0 if mod != 3), and use
CHECK_AVX(s, SSE_OPF_AVX_128 | SSE_OPF_AVX_V0_MEM);
It's okay not to move _all_ flags checks in the macros, but for example
here:
+ if (op6.ext_mask == CPUID_EXT_AVX
+ && (s->prefix & PREFIX_VEX) == 0) {
+ goto illegal_op;
+ }
+ if (op6.flags & SSE_OPF_AVX2) {
+ CHECK_AVX2(s);
+ }
+
if (b1) {
+ if (op6.flags & SSE_OPF_V0) {
+ CHECK_AVX_V0(s);
+ } else {
+ CHECK_AVX(s);
+ }
op1_offset = offsetof(CPUX86State,xmm_regs[reg]);
+
+ if (op6.flags & SSE_OPF_MMX) {
+ CHECK_AVX2_256(s);
+ }
there is a lot of room for using a flags-extended CHECK_AVX macro.
Also, SSE_OPF_V0 seems overloaded, because it means depending on the
place in the code:
- always 2-operand
- 2-operand except if SCALAR && !CMP
- 2-operand except if SCALAR && !CMP && has REPZ/REPNZ prefixes
It is not clear to me if the former overlaps with the last (i.e. if
there are any SCALAR && !CMP operations that are always 2-operand). If
so, please use different constants for all three; if not, please use a
different constant for the last, e.g. SSE_OPF_V0 and SSE_OPF_VEC_V0, so
that the difference is visible in the flags-extended CHECK_AVX macro.
Also related to overloading, here and in patch 37 there is code like this:
+ if (op7.flags & SSE_OPF_BLENDV && !(s->prefix & PREFIX_VEX)) {
+ /* Only VEX encodings are valid for these blendv opcodes */
+ goto illegal_op;
+ }
If this is for all SSE_OPF_BLENDV operations, it can be handled in the
flags-enabled CHECK_AVX() macro above. If it is only for some, it
should be a new flag SSE_OPF_VEX_ONLY.
Finally (replying here just to keep things together), patch 29 has "We
abuse the SSE_OPF_SCALAR flag to select the memory operand width
appropriately". Please don't; use a separate function that takes in "b"
and returns a bool, with just a switch statement in it.
+ CHECK_AVX(s);
+ scalar_op = (s->prefix & PREFIX_VEX)
+ && (op7.flags & SSE_OPF_SCALAR)
+ && !(op7.flags & SSE_OPF_CMP);
+ if (is_xmm && (op7.flags & SSE_OPF_MMX)) {
+ CHECK_AVX2_256(s);
+ }
I think the is_xmm check is always true here (inside case 0x03a: case
0x13a:, i.e. b is inside the 0x10..0x5f range)?
Paolo
