[PATCH] Support APX zero-upper
Cui, Lili
lili.cui@intel.com
Wed May 8 03:53:04 GMT 2024
> On 28.04.2024 12:54, Cui, Lili wrote:
> > --- a/gas/config/tc-i386.c
> > +++ b/gas/config/tc-i386.c
> > @@ -1920,7 +1920,7 @@ static INLINE bool need_evex_encoding (const
> insn_template *t)
> > return i.encoding == encoding_evex
> > || i.encoding == encoding_evex512
> > || (t->opcode_modifier.vex && i.encoding == encoding_egpr)
> > - || i.mask.reg;
> > + || i.mask.reg || t->opcode_modifier.zu;
> > }
>
> I wonder if this is really needed. Can you clarify why/how you found a need
> to add this?
>
Dropped.
> > @@ -3980,7 +3980,8 @@ is_apx_evex_encoding (void) {
> > return i.rex2 || i.tm.opcode_space == SPACE_EVEXMAP4 || i.has_nf
> > || (i.vex.register_specifier
> > - && (i.vex.register_specifier->reg_flags & RegRex2));
> > + && (i.vex.register_specifier->reg_flags & RegRex2))
> > + || i.tm.opcode_modifier.zu;
>
> This isn't needed - "i.tm.opcode_space == SPACE_EVEXMAP4" already covers
> all you need.
>
Dropped.
> > --- /dev/null
> > +++ b/gas/testsuite/gas/i386/x86-64-apx-zu-inval.s
> > @@ -0,0 +1,28 @@
> > +# Check illegal APX-ZU instructions
> > +
> > + .allow_index_reg
> > + .text
> > +_start:
> > + imulzub $0xa,%bl,%al
> > + imulzud $0xa,%ebx,%eax
> > + imulzu $0xa,%rbx,%rax
> > + imulzub $0xaaaa,%bl,%al
> > + imulzud $0xaaaa,%ebx,%eax
> > + imulzu $0xaaaa,%rbx,%rax
> > + imulzu $0xaaaa,%ebx,%rax
> > + imulzu $0xaaaa,%ebx,%rax
> > + setzuno %eax
> > + setzub %bx
> > + setzuae %r8w
> > + setzue %r9w
> > + setzune %r10d
> > + setzube %eax
> > + setzua %bx
> > + setzus %r18w
> > + setzuns %r19w
> > + setzup %r20d
> > + setzunp %r21w
> > + setzul %r22w
> > + setzuge %r23d
> > + setzule %r24w
> > + setzug %r25w
>
> How about having at least one case with a 64-bit register here, too?
> Further perhaps better also have one use of %ah, %ch, %dh, or %bh here.
>
Added.
> > --- a/opcodes/i386-opc.h
> > +++ b/opcodes/i386-opc.h
> > @@ -753,6 +753,9 @@ enum
> > /* Instrucion requires REX2 prefix. */
> > Rex2,
> >
> > + /* Support zero upper */
> > + ZU,
> > +
> > /* The last bitfield in i386_opcode_modifier. */
> > Opcode_Modifier_Num
> > };
> > @@ -800,6 +803,7 @@ typedef struct i386_opcode_modifier
> > unsigned int noegpr:1;
> > unsigned int nf:1;
> > unsigned int rex2:1;
> > + unsigned int zu:1;
> > } i386_opcode_modifier;
>
> Does this really need to be a new attribute? I would have expected a new
> OperandConstraint value would suffice.
>
I added ZU to OperandConstraint, but...
> > --- a/opcodes/i386-opc.tbl
> > +++ b/opcodes/i386-opc.tbl
> > @@ -399,8 +399,10 @@ imul, 0xfaf, i386,
> > Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Reg16|Reg32|Reg64|U
> imul,
> > 0xaf, APX_F, Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVexMap4|NF,
> {
> > Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 } imul,
> > 0x6b, i186, Modrm|CheckOperandSize|No_bSuf|No_sSuf, { Imm8S,
> > Reg16|Reg32|Reg64|Unspecified|BaseIndex, Reg16|Reg32|Reg64 } imul,
> > 0x6b, APX_F, Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVexMap4|NF,
> {
> > Imm8S, Reg16|Reg32|Reg64|Unspecified|BaseIndex,
> Reg16|Reg32|Reg64 }
> > +imulzu, 0x6b, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|NF|ZU,
> { Imm8S,
> > +Reg16|Unspecified|BaseIndex, Reg16 }
> > imul, 0x69, i186, Modrm|CheckOperandSize|No_bSuf|No_sSuf, {
> > Imm16|Imm32|Imm32S, Reg16|Reg32|Reg64|Unspecified|BaseIndex,
> > Reg16|Reg32|Reg64 } imul, 0x69, APX_F,
> > Modrm|CheckOperandSize|No_bSuf|No_sSuf|EVexMap4|NF, {
> > Imm16|Imm32|Imm32S, Reg16|Reg32|Reg64|Unspecified|BaseIndex,
> > Reg16|Reg32|Reg64 }
> > +imulzu, 0x69, APX_F, Modrm|No_bSuf|No_sSuf|EVexMap4|NF|ZU,
> { Imm16,
> > +Reg16|Unspecified|BaseIndex, Reg16 }
> > // imul with 2 operands mimics imul with 3 by putting the register in
> > // both i.rm.reg & i.rm.regmem fields. RegKludge enables this //
> > transformation.
>
> There's (once again) another adjustment wanted below here.
>
I found it conflicts with RegKludge, when adding these templates.
+imulzu, 0x6b, APX_F, Modrm|No_bSuf|No_sSuf|RegKludge|EVexMap4|NF|Zu, { Imm8S, Reg16 }
+imulzu, 0x69, APX_F, Modrm|No_bSuf|No_sSuf|RegKludge|EVexMap4|NF|Zu, { Imm16, Reg16 }
I think we need a new attribute for it.
Thanks,
Lili.
More information about the Binutils
mailing list