RE: [PATCH V5 8/9] Support APX NDD optimized encoding.

["Hu, Lin1" <lin1.hu@intel.com>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, January 5, 2024 10:36 PM
> To: Cui, Lili <lili.cui@intel.com>
> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; Hu, Lin1 <lin1.hu@intel.com>;
> binutils@sourceware.org
> Subject: Re: [PATCH V5 8/9] Support APX NDD optimized encoding.
> 
> On 28.12.2023 02:27, Cui, Lili wrote:
> > @@ -7754,6 +7804,60 @@ match_template (char mnem_suffix)
> >  	  i.memshift = memshift;
> >  	}
> >
> > +      /* If we can optimize a NDD insn to legacy insn, like
> > +	 add %r16, %r8, %r8 -> add %r16, %r8,
> > +	 add  %r8, %r16, %r8 -> add %r16, %r8, then rematch template.
> > +	 Note that the semantics have not been changed.  */
> > +      if (optimize
> > +	  && !i.no_optimize
> > +	  && i.vec_encoding != vex_encoding_evex
> > +	  && t + 1 < current_templates.end
> > +	  && !t[1].opcode_modifier.evex
> > +	  && t[1].opcode_space <= SPACE_0F38
> > +	  && t->opcode_modifier.vexvvvv == VexVVVV_DST
> > +	  && (i.types[i.operands - 1].bitfield.dword
> > +	      || i.types[i.operands - 1].bitfield.qword))
> 
> While you check the last operand's type here, ...
> 
> > +	{
> > +	  unsigned int match_dest_op = can_convert_NDD_to_legacy (t);
> > +
> > +	  if (match_dest_op != (unsigned int) ~0)
> > +	    {
> > +	      size_match = true;
> > +	      /* We ensure that the next template has the same input
> > +		 operands as the original matching template by the first
> > +		 opernd (ATT). To avoid someone support new NDD insns and
> > +		 put it in the wrong position.  */
> > +	      overlap0 = operand_type_and (i.types[0],
> > +					   t[1].operand_types[0]);
> > +	      if (t->opcode_modifier.d)
> > +		overlap1 = operand_type_and (i.types[0],
> > +					     t[1].operand_types[1]);
> > +	      if (!operand_type_match (overlap0, i.types[0])
> > +		  && (!t->opcode_modifier.d
> > +		      || !operand_type_match (overlap1, i.types[0])))
> > +		size_match = false;
> 
> .. why is it the first one's here? That may be a memory operand, which in AT&T
> mode cannot possibly have a size.
> 

These two places serve different purposes, I check last operand is for 8/16-bit operand don't have zero_upper (you mentioned). If the insn is a NDD insn, it's last operand must be a register operand, so it doesn't have the problem about memory operand.

The reason for checking the first operand is stated in the comment; it doesn't need to care about the size. It ensure the original matching template have same input like the next template (exclude the last operand). Because the original template is a NDD template and the next template is it's legacy version (If the future developer put them in right order).

BRs,
Lin