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

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

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, December 12, 2023 4:42 PM
> To: Hu, Lin1 <lin1.hu@intel.com>
> Cc: Lu, Hongjiu <hongjiu.lu@intel.com>; binutils@sourceware.org; Cui, Lili
> <lili.cui@intel.com>
> Subject: Re: [PATCH v3 8/9] Support APX NDD optimized encoding.
> 
> On 12.12.2023 04:18, Hu, Lin1 wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: Monday, December 11, 2023 8:28 PM
> >>
> >> On 24.11.2023 08:02, Cui, Lili wrote:
> >>> @@ -7675,6 +7727,61 @@ 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)
> >>> +	{
> >>> +	  unsigned int match_dest_op = can_convert_NDD_to_legacy (t);
> >>> +	  size_match = true;
> >>
> >> This would perhaps better ...
> >>
> >>> +	  if (match_dest_op != (unsigned int) ~0)
> >>> +	    {
> >>
> >> ... live here
> >>
> >
> > OK.
> >
> >>
> >>> +	      /* We ensure that the next template has the same input
> >>> +		 operands as the original matching template by the first
> >>> +		 opernd (ATT), thus avoiding the error caused by the wrong
> >> order
> >>> +		 of insns in i386.tbl.  */
> >>
> >> I'm sorry, but I (still) can't make sense of this last part of the
> >> comment, after the comma.
> >>
> >
> > I mean if someone support new NDD insns and put it in the wrong position, so
> the part will try to avoid to optimize the insn.
> 
> If this is about hypothetical new templates, that would want saying so in the
> comment. Thus clarifying that there's no functional effect right now.
> I wonder what H.J.'s view on such effectively dead code is.
> 
> However, there's a bigger problem with this patch as I realized only a few
> minutes ago when looking into Lili's reply on the NDD patch thread:
> NDD insns are implicitly zero-upper. Hence converting NDD to legacy insns needs
> to be limited to 32- and 64-bit operand size. For 8- and 16-bit operand size the
> results would differ, which isn't acceptable under any -O<n>. It may be okay to
> do such a conversion even for the smaller sizes, but then under a separate
> option explicitly permitting such a functional difference.
> 

We can constraint the optimization by add some conditions, like i.types[i.operands - 1].bitfield.dword || i.types[i.operands - 1].bitfield.qword.

BRs,
Lin