[PATCH v4] aarch64: Support for FEAT_PCDPHINT

Alice Carlotti alice.carlotti@arm.com
Mon Jul 7 11:38:16 GMT 2025


On Fri, Jun 13, 2025 at 05:21:08PM +0100, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> FEAT_PCDPHINT - Producer-consumer data placement hints - is an optional
> ISA extension that provides hint instructions to indicate:
> - a store in the current execution thread is generating data at a specific
> location, which a thread of execution on one or more other observers is
> waiting on.
> - the thread of execution on the current PE will read a location that may not
> yet have been written with the value to be consumed.
> 
> This extension introduces:
> - STSHH, a hint instruction, with operands (policies) keep and strm
> - PRFM *IR*, a new prefetch memory operand.
> ---
> Main changes from v3:
> * Rejects prfum ir
> * prfm ir does not change to prfum ir if the offset is not a multiple of
> 8, unlike the other prfm operands.
> * Updating error messages
> 
> Regression tested on aarch64-none-linux-gnu.
> 
> Ezra
> 
>  gas/config/tc-aarch64.c                  | 27 ++++++++++++++++++++++--
>  gas/testsuite/gas/aarch64/pcdphint-bad.d |  4 ++++
>  gas/testsuite/gas/aarch64/pcdphint-bad.l |  9 ++++++++
>  gas/testsuite/gas/aarch64/pcdphint-bad.s |  9 ++++++++
>  gas/testsuite/gas/aarch64/stshh.d        | 10 +++++++++
>  gas/testsuite/gas/aarch64/stshh.s        |  3 +++
>  gas/testsuite/gas/aarch64/system.d       |  6 +++---
>  include/opcode/aarch64.h                 |  3 +++
>  opcodes/aarch64-opc.c                    | 17 ++++++++++-----
>  opcodes/aarch64-tbl.h                    |  3 +++
>  10 files changed, 81 insertions(+), 10 deletions(-)
>  create mode 100644 gas/testsuite/gas/aarch64/pcdphint-bad.d
>  create mode 100644 gas/testsuite/gas/aarch64/pcdphint-bad.l
>  create mode 100644 gas/testsuite/gas/aarch64/pcdphint-bad.s
>  create mode 100644 gas/testsuite/gas/aarch64/stshh.d
>  create mode 100644 gas/testsuite/gas/aarch64/stshh.s
> 
> diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
> index acb56044fb5..c6e1808a2b1 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -4310,7 +4310,7 @@ parse_address_main (char **str, aarch64_opnd_info *operand,
>  	  operand->addr.offset.is_reg = 1;
>  	}
>        else if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT,
> -					 NORMAL_RESOLUTION))
> +                                         NORMAL_RESOLUTION))
>  	{
>  	  /* [Xn],#expr */
>  	  set_syntax_error (_("invalid expression in the address"));

This whitespace change is irrelevant to the patch, so should be dropped (it
also incorrectly replaces tabs with spaces).

> @@ -4607,7 +4607,9 @@ parse_hint_opt (const char *name, char **str,
>  	  && o->value != HINT_OPD_CSYNC)
>        || ((strcmp ("bti", name) == 0)
>  	  && (o->value != HINT_OPD_C && o->value != HINT_OPD_J
> -	      && o->value != HINT_OPD_JC)))
> +	      && o->value != HINT_OPD_JC))
> +      || ((strcmp ("stshh", name) == 0)
> +	  && (o->value != HINT_OPD_KEEP && o->value != HINT_OPD_STRM)))
>        return false;
>  
>    *str = q;
> @@ -8068,9 +8070,18 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>  
>  	case AARCH64_OPND_PRFOP:
>  	  val = parse_pldop (&str);
> +
> +	  if (opcode->iclass != ldst_pos && val == 0x18)
> +	    {
> +	      set_syntax_error (_("the IR prefetch operation is not valid for this"
> +	                          " instruction variant"));

Sorry for changing my mind on this, but I think the resulting error message
would read better if we changed this to:

"invalid prefetch operation (IR is not valid for this instruction variant)"

Does that sound better to you?

> +	      goto failure;
> +	    }
> +
>  	  /* This is an extension to accept a 0..31 immediate.  */
>  	  if (val == PARSE_FAIL)
>  	    po_imm_or_fail (0, 31);
> +
>  	  inst.base.operands[i].prfop = aarch64_prfops + val;
>  	  break;
>  
> @@ -8150,6 +8161,11 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>  	    goto failure;
>  	  break;
>  
> +	case AARCH64_OPND_STSHH_POLICY:
> +	  if (!parse_hint_opt (opcode->name, &str, &(info->hint_option)))
> +	    goto failure;
> +	  break;
> +
>  	case AARCH64_OPND_SME_ZAda_1b:
>  	case AARCH64_OPND_SME_ZAda_2b:
>  	case AARCH64_OPND_SME_ZAda_3b:
> @@ -9554,6 +9570,13 @@ fix_insn (fixS *fixP, uint32_t flags, offsetT value)
>  	  put_aarch64_insn (buf, new_inst->value);
>  	  break;
>  	}
> +      else if ((new_inst->opcode->op == OP_PRFM_POS)
> +	       && (new_inst->operands[0].prfop->value == 0x18))
> +	{
> +	  as_bad_where (fixP->fx_file, fixP->fx_line,
> +	    _("offset for `prfm ir' at operand 2 must be a multiple of 8"));
> +	  break;
> +	}

Could you add this check to try_to_encode_as_unscaled_ldst?  I think for
consistency we should stick with the existing error message in this case, so
just bail out when the condition fails, i.e.:

   if (new_op == OP_NIL)
     return false;
+ 
+  if (new_inst->opcode->op == OP_PRFM_POS
+      && new_inst->operands[0].prfop->value == 0x18)
+    return false;
 
   new_opcode = aarch64_get_opcode (new_op);
   gas_assert (new_opcode != NULL);


Apologies for the delayed review, but hopefully these should be the last revisions needed.

Thanks,
Alice

>        else if (new_inst->opcode->iclass == ldst_pos
>  	       && try_to_encode_as_unscaled_ldst (new_inst))
>  	{
> diff --git a/gas/testsuite/gas/aarch64/pcdphint-bad.d b/gas/testsuite/gas/aarch64/pcdphint-bad.d
> new file mode 100644
> index 00000000000..33373b45356
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/pcdphint-bad.d
> @@ -0,0 +1,4 @@
> +#name: Negative test of PCPHINT instructions.
> +#as: -march=armv8-a
> +#source: pcdphint-bad.s
> +#error_output: pcdphint-bad.l
> diff --git a/gas/testsuite/gas/aarch64/pcdphint-bad.l b/gas/testsuite/gas/aarch64/pcdphint-bad.l
> new file mode 100644
> index 00000000000..bf01b6b4aec
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/pcdphint-bad.l
> @@ -0,0 +1,9 @@
> +[^ :]+: Assembler messages:
> +[^ :]+:[0-9]+: Error: operand 1 must be an STSHH policy \(keep/strm\) -- `stshh kee'
> +[^ :]+:[0-9]+: Error: operand 1 must be an STSHH policy \(keep/strm\) -- `stshh strmm'
> +[^ :]+:[0-9]+: Error: operand 1 must be an STSHH policy \(keep/strm\) -- `stshh'
> +[^ :]+:[0-9]+: Error: constant expression required at operand 1 -- `prfm ir1234'
> +[^ :]+:[0-9]+: Error: invalid addressing mode at operand 2 -- `prfm ir,\[sp,x0,lsl#0\]'
> +[^ :]+:[0-9]+: Error: invalid addressing mode at operand 2 -- `prfm ir,a'
> +[^ :]+:[0-9]+: Error: the IR prefetch operation is not valid for this instruction variant at operand 1 -- `prfum ir,\[x0\]'
> +[^ :]+:[0-9]+: Error: offset for `prfm ir' at operand 2 must be a multiple of 8
> diff --git a/gas/testsuite/gas/aarch64/pcdphint-bad.s b/gas/testsuite/gas/aarch64/pcdphint-bad.s
> new file mode 100644
> index 00000000000..f49652217bb
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/pcdphint-bad.s
> @@ -0,0 +1,9 @@
> +a:
> +	stshh kee
> +	stshh strmm
> +	stshh
> +	prfm ir1234
> +	prfm ir, [sp, x0, lsl #0]
> +	prfm ir, a
> +	prfum ir, [x0]
> +	prfm ir, [x0, #15]
> diff --git a/gas/testsuite/gas/aarch64/stshh.d b/gas/testsuite/gas/aarch64/stshh.d
> new file mode 100644
> index 00000000000..095048d143f
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/stshh.d
> @@ -0,0 +1,10 @@
> +#as: -march=armv8-a
> +#objdump: -dr
> +
> +.*:     file format .*
> +
> +Disassembly of section \.text:
> +
> +0+ <.*>:
> +.*:	d503261f 	stshh	keep
> +.*:	d503263f 	stshh	strm
> diff --git a/gas/testsuite/gas/aarch64/stshh.s b/gas/testsuite/gas/aarch64/stshh.s
> new file mode 100644
> index 00000000000..1e306cd1a31
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/stshh.s
> @@ -0,0 +1,3 @@
> +a:
> +	stshh keep
> +	stshh strm
> diff --git a/gas/testsuite/gas/aarch64/system.d b/gas/testsuite/gas/aarch64/system.d
> index c1400771d43..c8716c0f317 100644
> --- a/gas/testsuite/gas/aarch64/system.d
> +++ b/gas/testsuite/gas/aarch64/system.d
> @@ -63,8 +63,8 @@ Disassembly of section \.text:
>  .*:	d50325bf 	hint	#0x2d
>  .*:	d50325df 	hint	#0x2e
>  .*:	d50325ff 	hint	#0x2f
> -.*:	d503261f 	hint	#0x30
> -.*:	d503263f 	hint	#0x31
> +.*:	d503261f 	(hint	#0x30|stshh	keep)
> +.*:	d503263f 	(hint	#0x31|stshh	strm)
>  .*:	d503265f 	hint	#0x32
>  .*:	d503267f 	hint	#0x33
>  .*:	d503269f 	hint	#0x34
> @@ -332,7 +332,7 @@ Disassembly of section \.text:
>  .*:	f9800c77 	prfm	pstslcstrm, \[x3, #24\]
>  .*:	d8000018 	prfm	#0x18, 0 <LABEL1>
>  .*: R_AARCH64_(P32_|)LD_PREL_LO19	LABEL1
> -.*:	f9800c78 	prfm	#0x18, \[x3, #24\]
> +.*:	f9800c78 	prfm	ir, \[x3, #24\]
>  .*:	d8000019 	prfm	#0x19, 0 <LABEL1>
>  .*: R_AARCH64_(P32_|)LD_PREL_LO19	LABEL1
>  .*:	f9800c79 	prfm	#0x19, \[x3, #24\]
> diff --git a/include/opcode/aarch64.h b/include/opcode/aarch64.h
> index dfe3f05820a..d5e1edcee3e 100644
> --- a/include/opcode/aarch64.h
> +++ b/include/opcode/aarch64.h
> @@ -688,6 +688,7 @@ enum aarch64_opnd
>    AARCH64_OPND_BARRIER_PSB,	/* Barrier operand for PSB.  */
>    AARCH64_OPND_BARRIER_GCSB,	/* Barrier operand for GCSB.  */
>    AARCH64_OPND_BTI_TARGET,	/* BTI {<target>}.  */
> +  AARCH64_OPND_STSHH_POLICY,	/* STSHH {<policy>}.  */
>    AARCH64_OPND_BRBOP,		/* BRB operation IALL or INJ in bit 5.  */
>    AARCH64_OPND_Rt_IN_SYS_ALIASES,	/* Defaulted and omitted Rt used in SYS aliases such as brb.  */
>    AARCH64_OPND_LSE128_Rt,	/* LSE128 <Xt1>.  */
> @@ -1770,6 +1771,8 @@ struct aarch64_inst
>  #define HINT_OPD_C	0x22
>  #define HINT_OPD_J	0x24
>  #define HINT_OPD_JC	0x26
> +#define HINT_OPD_KEEP	0x30
> +#define HINT_OPD_STRM	0x31
>  #define HINT_OPD_NULL	0x00
>  
>  
> diff --git a/opcodes/aarch64-opc.c b/opcodes/aarch64-opc.c
> index 4f0c71696fa..555270cae73 100644
> --- a/opcodes/aarch64-opc.c
> +++ b/opcodes/aarch64-opc.c
> @@ -596,6 +596,8 @@ const struct aarch64_name_value_pair aarch64_hint_options[] =
>    { "c",	HINT_OPD_C },		/* BTI C.  */
>    { "j",	HINT_OPD_J },		/* BTI J.  */
>    { "jc",	HINT_OPD_JC },		/* BTI JC.  */
> +  { "keep",	HINT_OPD_KEEP },	/* STSHH KEEP  */
> +  { "strm",	HINT_OPD_STRM },	/* STSHH STRM  */
>    { NULL,	HINT_OPD_NULL },
>  };
>  
> @@ -629,7 +631,7 @@ const struct aarch64_name_value_pair aarch64_prfops[32] =
>    { "pstl3strm", B(2, 3, 1) },
>    { "pstslckeep", B(2, 4, 0) },
>    { "pstslcstrm", B(2, 4, 1) },
> -  { NULL, 0x18 },
> +  { "ir", B(3, 1, 0) },
>    { NULL, 0x19 },
>    { NULL, 0x1a },
>    { NULL, 0x1b },
> @@ -5017,11 +5019,12 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
>        break;
>  
>      case AARCH64_OPND_PRFOP:
> -      if (opnd->prfop->name != NULL)
> -	snprintf (buf, size, "%s", style_sub_mnem (styler, opnd->prfop->name));
> +      if ((opnd->prfop->name == NULL)
> +          || (opcode->iclass != ldst_pos && opnd->prfop->value == 0x18))
> +        snprintf (buf, size, "%s",
> +                  style_imm (styler, "#0x%02x", opnd->prfop->value));
>        else
> -	snprintf (buf, size, "%s", style_imm (styler, "#0x%02x",
> -					      opnd->prfop->value));
> +        snprintf (buf, size, "%s", style_sub_mnem (styler, opnd->prfop->name));
>        break;
>  
>      case AARCH64_OPND_RPRFMOP:
> @@ -5071,6 +5074,10 @@ aarch64_print_operand (char *buf, size_t size, bfd_vma pc,
>  		  style_sub_mnem (styler, opnd->hint_option->name));
>        break;
>  
> +    case AARCH64_OPND_STSHH_POLICY:
> +      snprintf (buf, size, "%s", style_sub_mnem (styler, opnd->hint_option->name));
> +      break;
> +
>      case AARCH64_OPND_MOPS_ADDR_Rd:
>      case AARCH64_OPND_MOPS_ADDR_Rs:
>        snprintf (buf, size, "[%s]!",
> diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
> index 8b64eb07067..c93b73c12bb 100644
> --- a/opcodes/aarch64-tbl.h
> +++ b/opcodes/aarch64-tbl.h
> @@ -4588,6 +4588,7 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>    SME_INSN ("smstop",  0xd503407f, 0xfffff1ff, sme_stop,  0, OP1 (SME_SM_ZA), {}, F_SYS_WRITE, 0),
>    /* System.  */
>    CHK_INSN ("chkfeat", 0xd503251f, 0xffffffff, OP1 (X16), QL_I1X, 0),
> +  CORE_INSN ("stshh", 0xd503261f, 0xffffffdf, ic_system, 0, OP1 (STSHH_POLICY), {}, F_ALIAS),
>    CORE_INSN ("msr", 0xd500401f, 0xfff8f01f, ic_system, 0, OP2 (PSTATEFIELD, UIMM4), {}, F_SYS_WRITE),
>    CORE_INSN ("hint",0xd503201f, 0xfffff01f, ic_system, 0, OP1 (UIMM7), {}, F_HAS_ALIAS),
>    CORE_INSN ("nop", 0xd503201f, 0xffffffff, ic_system, 0, OP0 (), {}, F_ALIAS),
> @@ -7235,6 +7236,8 @@ const struct aarch64_opcode aarch64_opcode_table[] =
>        "the GCSB option name DSYNC")					\
>      Y(SYSTEM, hint, "BTI_TARGET", 0, F (),				\
>        "BTI targets j/c/jc")						\
> +    Y(SYSTEM, hint, "STSHH_POLICY", 0, F(),				\
> +      "an STSHH policy (keep/strm)")					\
>      Y(SYSTEM, imm, "BRBOP", 0, F(FLD_brbop),				\
>        "Branch Record Buffer operation operand")				\
>      Y(INT_REG, regno, "Rt_IN_SYS_ALIASES", 0, F(FLD_Rt),		\
> -- 
> 2.45.2
> 


More information about the Binutils mailing list