RE: [PATCH v3 6/9] Support APX NDD

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

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, March 26, 2024 3:06 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: Kong, Lingling <lingling.kong@intel.com>; binutils@sourceware.org; Lu,
> Hongjiu <hongjiu.lu@intel.com>
> Subject: Re: [PATCH v3 6/9] Support APX NDD
> 
> On 26.03.2024 03:04, Cui, Lili wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Friday, March 22, 2024 6:31 PM
> >> To: Cui, Lili <lili.cui@intel.com>; Lu, Hongjiu
> >> <hongjiu.lu@intel.com>
> >> Cc: Kong, Lingling <lingling.kong@intel.com>; binutils@sourceware.org
> >> Subject: Re: [PATCH v3 6/9] Support APX NDD
> >>
> >> On 22.03.2024 11:02, Jan Beulich wrote:
> >>> On 08.12.2023 15:12, Jan Beulich wrote:
> >>>> On 24.11.2023 08:02, Cui, Lili wrote:
> >>>>> +rol, 0xd0/0, APX_F,
> >>>>>
> >>
> +W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVex128|EVexMap4|NF, {
> >>>>> +Imm1,
> >>>>>
> >>
> +Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseInd
> >> ex,
> >>>>> +Reg8|Reg16|Reg32|Reg64 }
> >>>>>  rol, 0xd0/0, 0, W|Modrm|No_sSuf, { Imm1,
> >>>>>
> >>
> Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex
> >>  }
> >>>>> +rol, 0xc0/0, APX_F,
> >>>>>
> >>
> +W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVex128|EVexMap4|NF, {
> >>>>> +Imm8|Imm8S,
> >>>>>
> >>
> +Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseInd
> >> ex,
> >>>>> +Reg8|Reg16|Reg32|Reg64 }
> >>>>>  rol, 0xc0/0, i186, W|Modrm|No_sSuf, { Imm8|Imm8S,
> >>>>>
> >>
> Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex
> >>  }
> >>>>> +rol, 0xd2/0, APX_F,
> >>>>>
> >>
> +W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVex128|EVexMap4|NF, {
> >>>>> +ShiftCount,
> >>>>>
> >>
> +Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseInd
> >> ex,
> >>>>> +Reg8|Reg16|Reg32|Reg64 }
> >>>>>  rol, 0xd2/0, 0, W|Modrm|No_sSuf, { ShiftCount,
> >>>>>
> >>
> Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseIndex
> >>  }
> >>>>> +rol, 0xd0/0, APX_F,
> >>>>>
> >>
> +W|Modrm|No_sSuf|CheckOperandSize|DstVVVV|EVex128|EVexMap4|NF, {
> >>>>>
> >>
> +Reg8|Reg16|Reg32|Reg64|Byte|Word|Dword|Qword|Unspecified|BaseInd
> >> ex,
> >>>>> +Reg8|Reg16|Reg32|Reg64 }
> >>>>
> >>>> Didn't we agree to avoid adding this (and its sibling) template,
> >>>> for the omitted shift count being ambiguous? Consider
> >>>>
> >>>>     rol %cl, %al
> >>>>
> >>>> Is this a rotate by %cl, or a 1-bit NDD rotate?
> >>>
> >>> Btw, while this comment was taken into account for the "normal"
> >>> shifts and rotates, SHLD / SHRD still have this odd extra form.
> >>> There's not as much of an ambiguity there, but I think we should
> >>> demand %cl to be specified consistently across all respective APX insn
> forms.
> >>
> >> Actually the overall situation (for legacy shift insns) is even worse: For
> "normal"
> >> shifts / rotates, omitting the shift count means "$1", whereas for
> >> SHLD/SHRD it means "%cl". Prior to Lili's recent disassembler change
> >> it was also the case that only the "$1" would be omitted from output,
> >> but not the "%cl" (at least the disassembler is consistent now).
> >>
> >
> > I'm really confused about SHLD / SHRD, I think there is some problem with
> the legacy format. Normally we will omit $1, but we will not omit %cl. Should
> the opcode of the third item be 0fac?
> 
> I don't think so, but I have no idea what the origin of this omitted operand form
> is. At least if you look at the APX spec, it (imo wrongly) omits %cl as an operand,
> too. "Wrongly" not the least because that's not in line with the SDM.
> 
> Jan

Yes, I also found it this morning that the APX spec doesn't have %cl and will confirm it with the doc.

>
> > shrd, 0xfac, i386, Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Imm8,
> > Reg16|Reg32|Reg64, Reg16|Reg32|Reg64|Unspecified|BaseIndex } shrd,
> > 0xfad, i386, Modrm|CheckOperandSize|No_bSuf|No_sSuf, { ShiftCount,
> > Reg16|Reg32|Reg64, Reg16|Reg32|Reg64|Unspecified|BaseIndex } shrd,
> > 0xfad, i386, Modrm|CheckOperandSize|No_bSuf|No_sSuf, {
> > Reg16|Reg32|Reg64, Reg16|Reg32|Reg64|Unspecified|BaseIndex }
> >
> >
> > Thanks,
> > Lili.