public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H.J. Lu" <hjl.tools@gmail.com>
To: "Cui, Lili" <lili.cui@intel.com>
Cc: binutils@sourceware.org, jbeulich@suse.com
Subject: Re: [PATCH V5 7/9] Support APX pushp/popp
Date: Wed, 27 Dec 2023 17:56:01 -0800	[thread overview]
Message-ID: <ZYzVsaFLswCg1kXd@gmail.com> (raw)
In-Reply-To: <20231228012714.2989658-8-lili.cui@intel.com>

On Thu, Dec 28, 2023 at 01:27:12AM +0000, Cui, Lili wrote:
> gas/ChangeLog:
> 
> 	* config/tc-i386.c (process_operands): Handle "PUSHP/POPP requires
> 	rex2.w == 1."
> 	* testsuite/gas/i386/x86-64.exp: Add new test for PUSHP/POPP.
> 	* testsuite/gas/i386/x86-64-apx-pushp-popp-intel.d: New test.
> 	* testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l: Ditto.
> 	* testsuite/gas/i386/x86-64-apx-pushp-popp-inval.s: Ditto.
> 	* testsuite/gas/i386/x86-64-apx-pushp-popp.d: Ditto.
> 	* testsuite/gas/i386/x86-64-apx-pushp-popp.s: Ditto.
> 
> opcodes/ChangeLog:
> 
> 	* i386-dis.c (putop): print pushp and popp.
> 	* i386-opc.tbl: Added new insns.
> 	* i386-init.h : Regenerated.
> 	* i386-mnem.h : Regenerated.
> 	* i386-tbl.h: Regenerated.
> ---
>  gas/config/tc-i386.c                          |  3 +-
>  .../gas/i386/x86-64-apx-pushp-popp-intel.d    | 14 +++++
>  .../gas/i386/x86-64-apx-pushp-popp-inval.l    |  5 ++
>  .../gas/i386/x86-64-apx-pushp-popp-inval.s    |  7 +++
>  .../gas/i386/x86-64-apx-pushp-popp.d          | 14 +++++
>  .../gas/i386/x86-64-apx-pushp-popp.s          |  8 +++
>  gas/testsuite/gas/i386/x86-64.exp             |  3 +
>  opcodes/i386-dis.c                            | 55 ++++++++++++-------
>  opcodes/i386-opc.h                            |  2 +
>  opcodes/i386-opc.tbl                          |  3 +
>  10 files changed, 94 insertions(+), 20 deletions(-)
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-pushp-popp-intel.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.s
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-pushp-popp.d
>  create mode 100644 gas/testsuite/gas/i386/x86-64-apx-pushp-popp.s
> 
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index 8af98e435ef..640c6511f20 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3910,7 +3910,8 @@ is_apx_evex_encoding (void)
>  static INLINE bool
>  is_apx_rex2_encoding (void)
>  {
> -  return i.rex2 || i.rex2_encoding;
> +  return i.rex2 || i.rex2_encoding
> +	|| i.tm.opcode_modifier.operandconstraint == REX2_REQUIRED;
>  }
>  
>  static unsigned int
> diff --git a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-intel.d b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-intel.d
> new file mode 100644
> index 00000000000..44e3e96a5df
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-intel.d
> @@ -0,0 +1,14 @@
> +#as:
> +#objdump: -dw -Mintel
> +#name: x86_64 APX_F pushp popp insns (Intel disassembly)
> +#source: x86-64-apx-pushp-popp.s
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <_start>:
> +\s*[a-f0-9]+:\s*d5 08 50[	 ]+pushp  rax
> +\s*[a-f0-9]+:\s*d5 19 57[	 ]+pushp  r31
> +\s*[a-f0-9]+:\s*d5 08 58[	 ]+popp   rax
> +\s*[a-f0-9]+:\s*d5 19 5f[	 ]+popp   r31
> diff --git a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l
> new file mode 100644
> index 00000000000..c4d774b9673
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.l
> @@ -0,0 +1,5 @@
> +.* Assembler messages:
> +.*:4: Error: operand size mismatch for `pushp'
> +.*:5: Error: operand size mismatch for `popp'
> +.*:6: Error: operand size mismatch for `pushp'
> +.*:7: Error: operand size mismatch for `popp'
> diff --git a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.s b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.s
> new file mode 100644
> index 00000000000..28ed5d8145a
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp-inval.s
> @@ -0,0 +1,7 @@
> +# Check bytecode of APX_F pushp popp instructions with illegal instructions.
> +
> +	.text
> +	pushp %eax
> +	popp  %eax
> +	pushp (%rax)
> +	popp  (%rax)
> diff --git a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.d b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.d
> new file mode 100644
> index 00000000000..b20e5ba9a35
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.d
> @@ -0,0 +1,14 @@
> +#as:
> +#objdump: -dw
> +#name: x86_64 APX_F pushp popp insns
> +#source: x86-64-apx-pushp-popp.s
> +
> +.*: +file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <_start>:
> +\s*[a-f0-9]+:\s*d5 08 50[ 	]+pushp  %rax
> +\s*[a-f0-9]+:\s*d5 19 57[ 	]+pushp  %r31
> +\s*[a-f0-9]+:\s*d5 08 58[ 	]+popp   %rax
> +\s*[a-f0-9]+:\s*d5 19 5f[ 	]+popp   %r31
> diff --git a/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.s b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.s
> new file mode 100644
> index 00000000000..0ea66d0e70c
> --- /dev/null
> +++ b/gas/testsuite/gas/i386/x86-64-apx-pushp-popp.s
> @@ -0,0 +1,8 @@
> +# Check 64bit APX_F pushp popp instructions
> +
> +       .text
> + _start:
> +	pushp %rax
> +	pushp %r31
> +	popp  %rax
> +	popp  %r31
> diff --git a/gas/testsuite/gas/i386/x86-64.exp b/gas/testsuite/gas/i386/x86-64.exp
> index 0e7b5d0c073..1b13c52454e 100644
> --- a/gas/testsuite/gas/i386/x86-64.exp
> +++ b/gas/testsuite/gas/i386/x86-64.exp
> @@ -348,6 +348,9 @@ run_dump_test "x86-64-avx512dq-rcigrne"
>  run_dump_test "x86-64-apx-push2pop2"
>  run_dump_test "x86-64-apx-push2pop2-intel"
>  run_list_test "x86-64-apx-push2pop2-inval"
> +run_dump_test "x86-64-apx-pushp-popp"
> +run_dump_test "x86-64-apx-pushp-popp-intel"
> +run_list_test "x86-64-apx-pushp-popp-inval"
>  run_dump_test "x86-64-avx512dq-rcigru-intel"
>  run_dump_test "x86-64-avx512dq-rcigru"
>  run_dump_test "x86-64-avx512dq-rcigrz-intel"
> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
> index 9daef6fa9fd..e851fb376d9 100644
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -301,6 +301,9 @@ struct dis_private {
>  #define EVEX_len_used 2
>  
>  
> +/* {rex2} is not printed when the REX2_SPECIAL is set.  */
> +#define REX2_SPECIAL 16
> +
>  /* Flags stored in PREFIXES.  */
>  #define PREFIX_REPZ 1
>  #define PREFIX_REPNZ 2
> @@ -1924,23 +1927,23 @@ static const struct dis386 dis386[] = {
>    { "dec{S|}",		{ RMeSI }, 0 },
>    { "dec{S|}",		{ RMeDI }, 0 },
>    /* 50 */
> -  { "push{!P|}",		{ RMrAX }, 0 },
> -  { "push{!P|}",		{ RMrCX }, 0 },
> -  { "push{!P|}",		{ RMrDX }, 0 },
> -  { "push{!P|}",		{ RMrBX }, 0 },
> -  { "push{!P|}",		{ RMrSP }, 0 },
> -  { "push{!P|}",		{ RMrBP }, 0 },
> -  { "push{!P|}",		{ RMrSI }, 0 },
> -  { "push{!P|}",		{ RMrDI }, 0 },
> +  { "push!P",		{ RMrAX }, 0 },
> +  { "push!P",		{ RMrCX }, 0 },
> +  { "push!P",		{ RMrDX }, 0 },
> +  { "push!P",		{ RMrBX }, 0 },
> +  { "push!P",		{ RMrSP }, 0 },
> +  { "push!P",		{ RMrBP }, 0 },
> +  { "push!P",		{ RMrSI }, 0 },
> +  { "push!P",		{ RMrDI }, 0 },
>    /* 58 */
> -  { "pop{!P|}",		{ RMrAX }, 0 },
> -  { "pop{!P|}",		{ RMrCX }, 0 },
> -  { "pop{!P|}",		{ RMrDX }, 0 },
> -  { "pop{!P|}",		{ RMrBX }, 0 },
> -  { "pop{!P|}",		{ RMrSP }, 0 },
> -  { "pop{!P|}",		{ RMrBP }, 0 },
> -  { "pop{!P|}",		{ RMrSI }, 0 },
> -  { "pop{!P|}",		{ RMrDI }, 0 },
> +  { "pop!P",		{ RMrAX }, 0 },
> +  { "pop!P",		{ RMrCX }, 0 },
> +  { "pop!P",		{ RMrDX }, 0 },
> +  { "pop!P",		{ RMrBX }, 0 },
> +  { "pop!P",		{ RMrSP }, 0 },
> +  { "pop!P",		{ RMrBP }, 0 },
> +  { "pop!P",		{ RMrSI }, 0 },
> +  { "pop!P",		{ RMrDI }, 0 },
>    /* 60 */
>    { X86_64_TABLE (X86_64_60) },
>    { X86_64_TABLE (X86_64_61) },
> @@ -9783,9 +9786,10 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
>  
>    /* Check if the REX2 prefix is used.  */
>    if (ins.last_rex2_prefix >= 0
> -      && ((ins.rex2 & 0x7) ^ (ins.rex2_used & 0x7)) == 0
> -      && (ins.rex ^ ins.rex_used) == 0
> -      && (ins.rex2 & 0x7))
> +      && ((ins.rex2 & REX2_SPECIAL)
> +	  || (((ins.rex2 & 7) ^ (ins.rex2_used & 7)) == 0
> +	      && (ins.rex ^ ins.rex_used) == 0
> +	      && (ins.rex2 & 7))))
>      ins.all_prefixes[ins.last_rex2_prefix] = 0;
>  
>    /* Check if the SEG prefix is used.  */
> @@ -10632,6 +10636,19 @@ putop (instr_info *ins, const char *in_template, int sizeflag)
>  	case 'P':
>  	  if (l == 0)
>  	    {
> +	      if (!cond && ins->last_rex2_prefix >= 0 && (ins->rex & REX_W))
> +		{
> +		  /* For pushp and popp, p is printed and do not print {rex2}
> +		     for them.  */
> +		  *ins->obufp++ = 'p';
> +		  ins->rex2 |= REX2_SPECIAL;
> +		  break;
> +		}
> +
> +	      /* For "!P" print nothing else in Intel syntax.  */
> +	      if (!cond && ins->intel_syntax)
> +		break;
> +
>  	      if ((ins->modrm.mod == 3 || !cond)
>  		  && !(sizeflag & SUFFIX_ALWAYS))
>  		break;
> diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
> index 9e8c827b934..8db6c51538a 100644
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -579,6 +579,8 @@ enum
>    /* Instrucion requires that destination must be distinct from source
>       registers.  */
>  #define DISTINCT_DEST 9
> +  /* Instrucion requires REX2 prefix.  */
> +#define REX2_REQUIRED 10
>    OperandConstraint,
>    /* instruction ignores operand size prefix and in Intel mode ignores
>       mnemonic size suffix check.  */
> diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
> index 900ca36d286..edd9f73ae22 100644
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -85,6 +85,7 @@
>  #define RegKludge         OperandConstraint=REG_KLUDGE
>  #define SwapSources       OperandConstraint=SWAP_SOURCES
>  #define Ugh               OperandConstraint=UGH
> +#define Rex2              OperandConstraint=REX2_REQUIRED
>  
>  #define ATTSyntax         Dialect=ATT_SYNTAX
>  #define ATTMnemonic       Dialect=ATT_MNEMONIC
> @@ -229,6 +230,7 @@ push, 0x68, i186&No64, DefaultSize|No_bSuf|No_sSuf|No_qSuf, { Imm16|Imm32 }
>  push, 0x6, No64, DefaultSize|No_bSuf|No_sSuf|No_qSuf, { SReg }
>  // In 64bit mode, the operand size is implicitly 64bit.
>  push, 0x50, x64, No_bSuf|No_lSuf|No_sSuf|NoRex64, { Reg16|Reg64 }
> +pushp, 0x50, APX_F, No_bSuf|No_wSuf|No_lSuf|No_sSuf|Rex2, { Reg64 }
>  push, 0xff/6, x64, Modrm|DefaultSize|No_bSuf|No_lSuf|No_sSuf|NoRex64, { Reg16|Reg64|Unspecified|BaseIndex }
>  push, 0x6a, x64, DefaultSize|No_bSuf|No_lSuf|No_sSuf|NoRex64, { Imm8S }
>  push, 0x68, x64, DefaultSize|No_bSuf|No_lSuf|No_sSuf|NoRex64, { Imm16|Imm32S }
> @@ -242,6 +244,7 @@ pop, 0x8f/0, No64, Modrm|DefaultSize|No_bSuf|No_sSuf|No_qSuf, { Reg16|Reg32|Unsp
>  pop, 0x7, No64, DefaultSize|No_bSuf|No_sSuf|No_qSuf, { SReg }
>  // In 64bit mode, the operand size is implicitly 64bit.
>  pop, 0x58, x64, No_bSuf|No_lSuf|No_sSuf|NoRex64, { Reg16|Reg64 }
> +popp, 0x58, APX_F, No_bSuf|No_wSuf|No_lSuf|No_sSuf|Rex2, { Reg64 }
>  pop, 0x8f/0, x64, Modrm|DefaultSize|No_bSuf|No_lSuf|No_sSuf|NoRex64, { Reg16|Reg64|Unspecified|BaseIndex }
>  pop, 0xfa1, x64, DefaultSize|No_bSuf|No_lSuf|No_sSuf|NoRex64, { SReg }
>  
> -- 
> 2.25.1
> 

OK.

Thanks.

H.J.

  reply	other threads:[~2023-12-28  1:56 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-28  1:27 [PATCH V5 0/9] Support Intel APX EGPR Cui, Lili
2023-12-28  1:27 ` [PATCH V5 1/9] Support APX GPR32 with rex2 prefix Cui, Lili
2023-12-28  1:53   ` H.J. Lu
2024-01-04  8:02     ` Jan Beulich
2024-01-04 11:27       ` Cui, Lili
2024-01-05 14:45   ` Jan Beulich
2024-01-08  3:41     ` Cui, Lili
2023-12-28  1:27 ` [PATCH V5 2/9] Created an empty EVEX_MAP4_ sub-table for EVEX instructions Cui, Lili
2023-12-28  1:54   ` H.J. Lu
2023-12-28  1:27 ` [PATCH V5 3/9] Support APX GPR32 with extend evex prefix Cui, Lili
2023-12-28  1:54   ` H.J. Lu
2023-12-28 13:48     ` Cui, Lili
2023-12-28  1:27 ` [PATCH V5 4/9] Add tests for " Cui, Lili
2023-12-28  1:54   ` H.J. Lu
2023-12-28  1:27 ` [PATCH V5 5/9] Support APX NDD Cui, Lili
2023-12-28  1:55   ` H.J. Lu
2023-12-28  1:27 ` [PATCH V5 6/9] Support APX Push2/Pop2 Cui, Lili
2023-12-28  1:55   ` H.J. Lu
2023-12-28  1:27 ` [PATCH V5 7/9] Support APX pushp/popp Cui, Lili
2023-12-28  1:56   ` H.J. Lu [this message]
2023-12-28  1:27 ` [PATCH V5 8/9] Support APX NDD optimized encoding Cui, Lili
2023-12-28  1:56   ` H.J. Lu
2024-01-05 14:36   ` Jan Beulich
2024-01-08  2:49     ` Hu, Lin1
2023-12-28  1:27 ` [PATCH V5 9/9] Support APX JMPABS for disassembler Cui, Lili
2023-12-28  1:56   ` H.J. Lu
2024-01-05 12:08   ` Jan Beulich
2024-01-08  2:32     ` Hu, Lin1
2024-01-08  7:41       ` Jan Beulich
2024-01-08  7:44         ` Hu, Lin1

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZYzVsaFLswCg1kXd@gmail.com \
    --to=hjl.tools@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    --cc=lili.cui@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).