Re: [PATCH 10/10] Support Intel PREFETCHI

["H.J. Lu" <hjl.tools@gmail.com>]

On Tue, Oct 25, 2022 at 8:52 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 25.10.2022 15:03, Cui, Lili wrote:
> > @@ -6363,6 +6363,13 @@ check_VecOperands (const insn_template *t)
> >           i.types[op].bitfield.disp8 = 0;
> >         }
> >      }
> > +  /* Check if IP-relative addressing requirements can be satisfied.  */
> > +  for (op = 0; op < t->operands; op++)
> > +    {
> > +      if (t->operand_types[op].bitfield.instance == RegRIP
> > +       && !(i.base_reg && i.base_reg->reg_num == RegIP))
> > +     as_warn (_("only support RIP-relative address"));
> > +    }
>
> I don't think this function should be checking legacy encoded non-vector
> insn properties. I guess this wants moving into the caller; ideally you'd
> find an existing loop over all operands where could stick this. This
> check, after all, can be done any time after template matching, even as
> late as e.g. in build_modrm_byte().

This is a special case for instruction prefetch.  We can simply check

  if (i.tm.cpu_flags.bitfield.cpuprefetchi
      && !(i.base_reg && i.base_reg->reg_num == RegIP))

> > --- a/opcodes/i386-opc.tbl
> > +++ b/opcodes/i386-opc.tbl
> > @@ -40,6 +40,7 @@
> >  #define RegC Instance=RegC
> >  #define RegD Instance=RegD
> >  #define RegB Instance=RegB
> > +#define RegRIP Instance=RegRIP
>
> This and ...
>
> > @@ -3323,3 +3324,10 @@ rdmsrlist, 0xf20f01c6, None, CpuMSRLIST|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|N
> >  wrmsrlist, 0xf30f01c6, None, CpuMSRLIST|Cpu64, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, {}
> >
> >  // MSRLIST instructions end.
> > +
> > +// PREFETCHI instructions.
> > +
> > +prefetchit0, 0xf18, 0x7, CpuPREFETCHI|Cpu64, Modrm|Anysize|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegRIP|BaseIndex }
> > +prefetchit1, 0xf18, 0x6, CpuPREFETCHI|Cpu64, Modrm|Anysize|IgnoreSize|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { RegRIP|BaseIndex }
>
> ... its use here is certainly an abuse: The meaning is that plain %rip is
> allowed as an operand here. But I take it that you checked thoroughly that
> there's not going to be any undue side effect, in which case this might be
> acceptable. A better solution would imo be to add a CPU (or opcode, but
> that's more expensive) conditional to the check in tc-i386.c.
>
> Jan



-- 
H.J.