[PATCH 2/2] Support APX zero-upper

[Jan Beulich]

On 15.05.2024 08:31, Cui, Lili wrote:
> gas/ChangeLog:
> 
>         * config/tc-i386.c (build_apx_evex_prefix): Handle ZU.
>         * testsuite/gas/i386/x86-64.exp: Added new tests for ZU.
>         * testsuite/gas/i386/x86-64.exp: Added new tests for ZU.
>         * testsuite/gas/i386/x86-64-apx-zu-intel.d: New test.
>         * testsuite/gas/i386/x86-64-apx-zu-inval.l: Ditto.
>         * testsuite/gas/i386/x86-64-apx-zu-inval.s: Ditto.
>         * testsuite/gas/i386/x86-64-apx-zu.d: Ditto.
>         * testsuite/gas/i386/x86-64-apx-zu.s: Ditto.
> 
> opcodes/ChangeLog:
> 
>         * i386-dis-evex-prefix.h: Handle PREFIX_EVEX_MAP4_40 ~
>         PREFIX_EVEX_MAP4_4F.
>         * i386-dis-evex.h: Ditto.
>         * i386-dis.c (struct dis386): Add new micro 'ZU'.
>         (putop): Handle %ZU.
>         * i386-gen.c: Added ZU.
>         * i386-opc.h: Ditto.
>         * i386-opc.tbl: Added new templates to support ZU.
> ---
>  gas/config/tc-i386.c                         |   5 +-
>  gas/testsuite/gas/i386/x86-64-apx-zu-intel.d |  78 +++++++++++++
>  gas/testsuite/gas/i386/x86-64-apx-zu-inval.l |  25 +++++
>  gas/testsuite/gas/i386/x86-64-apx-zu-inval.s |  28 +++++
>  gas/testsuite/gas/i386/x86-64-apx-zu.d       |  78 +++++++++++++
>  gas/testsuite/gas/i386/x86-64-apx-zu.s       |  73 ++++++++++++
>  gas/testsuite/gas/i386/x86-64.exp            |   3 +
>  opcodes/i386-dis-evex-prefix.h               | 112 +++++++++++++++++++
>  opcodes/i386-dis-evex.h                      |  36 +++---
>  opcodes/i386-dis.c                           |  35 ++++++
>  opcodes/i386-gen.c                           |   1 +
>  opcodes/i386-opc.h                           |   4 +
>  opcodes/i386-opc.tbl                         |   6 +
>  13 files changed, 464 insertions(+), 20 deletions(-)
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-zu-intel.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-zu-inval.l
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-zu-inval.s
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-zu.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-zu.s
> 

This is (at least) v2, isn't it? Just to repeat an earlier request: Please
provide a brief summary of what has changed from the prior version, to aid
reviewers.

> @@ -10877,6 +10894,24 @@ putop (instr_info *ins, const char *in_template, int sizeflag)
>  	      abort ();
>  	    }
>  	  break;
> +	case 'U':
> +	  if (l == 1 && (last[0] == 'Z'))
> +	    {
> +	      /* Although IMUL/SETcc does not support NDD, the EVEX.ND bit is
> +		 used to control whether its destination register has its upper
> +		 bits zeroed when OSIZE is 16b/8b.  */
> +	      if (ins->vex.nd)
> +		{
> +		  oappend (ins, "zu");
> +		  /* When we print zu for the EVEX instruction, we no longer
> +		     need prefix {evex}. */
> +		  if (evex_printed == true && startswith (ins->obufp, "{evex}"))
> +		    ins->obufp += 6;

Is this (a) necessary and (b) correct? I ask in particular because I'd have
expected an increment by 7 to also skip the separating blank. Things being
okay with 6 suggests this path isn't really taken (or tested). The extra
startswith() check also looks somewhat questionable: If evex_printed is
true, there ought to be an {evex} prefix. It may not be at the start, but
then you'd still want to zap it (if any zapping is needed here in the first
place).

As to necessary: Isn't what you're trying to undo here the output of {evex}
pretty close to the top of the function? That's under !ins->vex.nd, while
here you check that ins->vex.nd is true. IOW both can't happen at the same
time. It also can't be the one output for %NF, as that's evex-from-vex only.

Assuming it's still needed for a reason I can't spot right now: I also have
to admit I'm slightly wary of such an increment. Elsewhere we always
re-arrange buffer contents, I think.

> @@ -529,6 +533,8 @@ loopne, 0xe0, x64, JumpByte|No_bSuf|No_wSuf|No_sSuf|NoRex64, { Disp8 }
>  
>  // Set byte on flag instructions.
>  set<cc>, 0xf9<cc:opc>/0, i386, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf, { Reg8|Unspecified|BaseIndex }
> +set<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|VexWIG|EVexMap4, { Reg8 }
> +setzu<cc>, 0xf24<cc:opc>/0, APX_F, Modrm|No_wSuf|No_lSuf|No_sSuf|No_qSuf|VexWIG|EVexMap4|ZU, { Reg8 }

If you really insist on not adding the extended form right away that I've
been keeping to ask for, please say a sentence or two towards that in the
(sadly so far again empty) description.

Jan