[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