On 7/31/19 6:47 PM, Richard Henderson wrote:
> I suppose there aren't so many different combinations, but did you consider
> separate callbacks per operand? If you have
>
> typedef unsigned (*gen_offset)(CPUX86State *, DisasContext *, int);
>
> static unsigned offset_Pq(CPUX86State *env, DisasContext *s, int modrm)
> {
> int reg = (modrm >> 3) & 7; /* Ignore REX_R */
> return offsetof(CPUX86State, fpregs[reg].mmx);
> }
>
> static unsigned offset_Qq(CPUX86State *env, DisasContext *s, int modrm)
> {
> int mod = (modrm >> 6) & 3;
> unsigned ret;
>
> if (mod == 3) {
> int rm = modrm & 7; /* Ignore REX_B */
> ret = offsetof(CPUX86State, fpregs[rm].mmx);
> } else {
> ret = offsetof(CPUX86State, mmx_t0);
> gen_lea_modrm(env, s, modrm);
> gen_ldq_env_A0(s, ret);
> }
> return ret;
> }
>
> static unsigned offset_Vx(CPUX86State *env, DisasContext *s, int modrm)
> {
> int reg = ((modrm >> 3) & 7) | REX_R(s);
> return offsetof(CPUX86State, xmm_regs[reg]);
> }
>
> static unsigned offset_Wx(CPUX86State *env, DisasContext *s, int modrm)
> {
> int mod = (modrm >> 6) & 3;
> unsigned ret;
>
> if (mod == 3) {
> int rm = (modrm & 7) | REX_B(s);
> ret = offsetof(CPUX86State, xmm_regs[rm]);
> } else {
> ret = offsetof(CPUX86State, xmm_t0);
> gen_lea_modrm(env, s, modrm);
> gen_ldo_env_A0(s, ret);
> }
> return ret;
> }
>
> static unsigned offset_Hx(CPUX86State *env, DisasContext *s, int modrm)
> {
> return offsetof(CPUX86State, xmm_regs[s->vex_v]);
> }
>
> Then you can have
>
> #define GEN_GVEC_3(OP0, OP1, OP2, OPRSZ, MAXSZ)
> static void gen_gvec_ld_modrm_##OP0##OP1##OP2(CPUX86State *env, \
> DisasContext *s, int modrm, unsigned vece, gen_gvec_2_fp_t gen) \
> { \
> int ofd = offset_##OP0(env, s, modrm); \
> int of1 = offset_##OP1(env, s, modrm); \
> int of2 = offset_##OP2(env, s, modrm); \
> gen(vece, opd, opa, opb, OPRSZ, MAXSZ); \
> }
>
> GEN_GVEC_3(Pq, Pq, Qq, sizeof(MMXReg), sizeof(MMXReg))
> GEN_GVEC_3(Vx, Vx, Wx, sizeof(XMMReg), max_vec_size(s))
> GEN_GVEC_3(Vx, Hx, Wx, sizeof(XMMReg), max_vec_size(s))
>
> The PqPqQq and VxVxWx sub-strings aren't quite canonical, but imo a better fit
> to the actual format of the instruction, with 2 inputs and 1 output.
Funny, I had a similar idea and converged to almost identical
solution. This will be part of v2.
> You can also do
>
> GEN_GVEC_3(Pq, Qq, Pq, sizeof(MMXReg), sizeof(MMXReg))
>
> for those rare "reversed" operations like PANDN. Now you don't need to carry
> around the OPCTL argument, which I initially found non-obvious.
Yup, solves the problem nicely and more clearly.
> I initially thought you'd be able to infer maxsz from the set of arguments,
> but
> since there are vex encoded operations that do not use vex.vvvv that is not
> always the case. Thus I suggest
>
> static size_t max_vec_size(DisasContext *s)
> {
> if (s->prefixes & PREFIX_VEX) {
> /*
> * TODO: When avx512 is supported and enabled, sizeof(ZMMReg).
> * In the meantime don't waste time zeroing data that is not
> * architecturally present.
> */
> return sizeof(YMMReg);
> } else {
> /* Without vex encoding, only the low 128 bits are modified. */
> return sizeof(XMMReg);
> }
> }
Looks good.
-Jan
signature.asc
Description: OpenPGP digital signature
