RE: [PATCH] x86: Optimize the encoder of the vvvv register

["Cui, Lili" <lili.cui@intel.com>]

> >>> +psllw<mmx>, 0x<mmx:pfx>0f71/6, <mmx:cpu>,
> >> Modrm|<mmx:scal>|NoSuf, {
> >>> +Imm8, <mmx:reg> }
> >>
> >> This is not a scalar instruction, hence "scal" as a template
> >> parameter name is misleading. It's not really clear to me anyway why
> >> this needs fiddling with - there was no SwapSources here, and none of
> >> its siblings are being touched either.
> >>
> >
> > 'psllw' has an extended opcode and two non-immediate operands, to
> delete the corresponding code below.
> >
> > -  if (i.tm.extension_opcode != None)
> > -    {
> > -      if (dest != source)
> > -       v = dest;
> > -      dest = ~0;
> > -    }
> 
> Yet how's psllw different from, say, psraw or pslld?
> 

Good question, psllw, psll<dq>, psraw, psrad, psrlw, psrl and psrl<dq> all require DstVVVV and I'll add test cases for them.

> >> To help recognizing such anomalies (possible problems), could I talk
> >> you into splitting the patch in two pieces? First a purely mechanical
> >> one introducing (perhaps simply as an alias of VexVVVV) / using
> >> Src1VVVV wherever it is meant to be used. Then the remaining changes,
> >> with a much smaller diff on the actual opcode templates, in the 2nd one.
> >>
> >
> > How about splitting into 3 patches:
> > 1. Introduce VexVVVV , Src1VVVV and Src2VVVV.
> > 2. Replace SwapSources.
> > 3. Replace extension_opcode part.
> 
> Fundamentally fine with me, just that it would seem to me that in such a 1st
> patch Src2VVVV would end up unused. Hence it would appear more logical
> to me to introduce that only when needed, i.e. in patch 2.
> 

Ok.

Thanks,
Lili.