Re: [Qemu-devel] [RFC PATCH v1 08/22] target/i386: reimplement (V)PAND, (V)ANDPS, (V)ANDPD

[Richard Henderson]

On 7/31/19 1:27 PM, Aleksandar Markovic wrote:
> 
> 
> On Wed, Jul 31, 2019 at 9:36 PM Richard Henderson <address@hidden
> <mailto:address@hidden>> wrote:
> 
>     On 7/31/19 10:56 AM, Jan Bobek wrote:
>     > +#define gen_pand_mm(env, s, modrm)   gen_gvec_ld_modrm_mm  ((env), (s),
>     (modrm), MO_64, tcg_gen_gvec_and, 0112)
>     > +#define gen_pand_xmm(env, s, modrm)  gen_gvec_ld_modrm_xmm ((env), (s),
>     (modrm), MO_64, tcg_gen_gvec_and, 0112)
>     > +#define gen_vpand_xmm(env, s, modrm) gen_gvec_ld_modrm_vxmm((env), (s),
>     (modrm), MO_64, tcg_gen_gvec_and, 0123)
>     > +#define gen_vpand_ymm(env, s, modrm) gen_gvec_ld_modrm_vymm((env), (s),
>     (modrm), MO_64, tcg_gen_gvec_and, 0123)
>     > +#define gen_andps_xmm  gen_pand_xmm
>     > +#define gen_vandps_xmm gen_vpand_xmm
>     > +#define gen_vandps_ymm gen_vpand_ymm
>     > +#define gen_andpd_xmm  gen_pand_xmm
>     > +#define gen_vandpd_xmm gen_vpand_xmm
>     > +#define gen_vandpd_ymm gen_vpand_ymm
> 
> 
>     Why all of these extra defines?
> 
> 
> Because of code clarity and safety, I would say.
> 
> This line:
> 
> case 0x54 | M_0F:                  gen_andps_xmm(env, s, modrm); return;
> 
> looks much clearer than this one:
> 
> case 0x54 | M_0F:                  gen_gvec_ld_modrm_mm(env, s, modrm, MO_64,
> tcg_gen_gvec_and, 0112)
> 
> and such organization is also much less prone to copy/paste bugs etc.

I'm not convinced.  These macros will be used exactly once.  Because there is
no reuse, there is no added safety.  There is only the chance of a typo in a
location removed from the actual use within the switch.

I agree that the goal is to minimize any replication per case.  But I don't
think this particular arrangement of macros is the way to accomplish that.


r~