[PATCH v3] aarch64: Support for FEAT_PCDPHINT
Alice Carlotti
alice.carlotti@arm.com
Thu May 8 18:04:49 GMT 2025
On Thu, May 08, 2025 at 02:18:27PM +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.
> ---
> I've addressed the comments from v2.
>
> > PRFM (immediate) doesn't support type IR, so this line shouldn't have changed.
> > I think you've also mistakenly added IR support to PRFUM. Both of these (and
> > PRFM (register)) should have tests that we reject the IR type, although I
> > actually already have a test for prfum #18 in a patch I'm currently working on.
>
> PRFM (immediate) does support ir, PRFM (register) and PRFM (literal) don't.
> I've added checks in parse_operands and aarch64_print_operand to handle
> this. AFAIK, ir should only be accepted from PRFM (immediate), the
> otheres can't but they can still take 0x18. Likewise for printing, it
> depends if it is PRFM (immediate) or not. Please let me know if there is
> a better way of handling this.
>
> Ezra
> gas/config/tc-aarch64.c | 19 ++++++++++++++++++-
> gas/testsuite/gas/aarch64/pcdphint-bad.d | 4 ++++
> gas/testsuite/gas/aarch64/pcdphint-bad.l | 7 +++++++
> gas/testsuite/gas/aarch64/pcdphint-bad.s | 8 ++++++++
> gas/testsuite/gas/aarch64/stshh.d | 10 ++++++++++
> gas/testsuite/gas/aarch64/stshh.s | 6 ++++++
> 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, 74 insertions(+), 9 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..6a8ce0b380e 100644
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -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,19 @@ parse_operands (char *str, const aarch64_opcode *opcode)
>
> case AARCH64_OPND_PRFOP:
> val = parse_pldop (&str);
> +
> + if ((opcode->iclass != ldst_pos || opcode->op != OP_PRFM_POS)
I don't think you need the OP_PRFM_POS check here; removing this would make the
condition match your check in aarch64-opc.c.
> + && val == 0x18)
> + {
> + set_syntax_error (_("the specified option is not accepted for "
> + " this PRFM variant"));
Can this error message be more specific - perhaps "the IR prefetch operation is
not valid in this instruction variant".
(Note that this operand is used for both PRFM and PRFUM mnemonics.)
> + 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 +8162,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:
> 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..ca5d6cd982f
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/pcdphint-bad.l
> @@ -0,0 +1,7 @@
> +[^ :]+: Assembler messages:
> +[^ :]+:[0-9]+: Error: operand 1 must be STSHH policies keep/strm -- `stshh kee'
> +[^ :]+:[0-9]+: Error: operand 1 must be STSHH policies keep/strm -- `stshh strmm'
> +[^ :]+:[0-9]+: Error: operand 1 must be STSHH policies keep/strm -- `stshh'
Sorry, I didn't notice this before, but these messages have bad grammar.
Specifying the operand description as "an STSHH policy (keep/strm)" should fix
the grammar.
(There are similar preexisting issues with the other hint descriptions.)
> +[^ :]+:[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'
> diff --git a/gas/testsuite/gas/aarch64/pcdphint-bad.s b/gas/testsuite/gas/aarch64/pcdphint-bad.s
> new file mode 100644
> index 00000000000..696c1fb55e6
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/pcdphint-bad.s
> @@ -0,0 +1,8 @@
> + stshh kee
> + stshh strmm
> + stshh
> +
> +a:
> + prfm ir1234
> + prfm ir, [sp, x0, lsl #0]
> + prfm ir, a
Can you add a prfum test here as well? E.g. prfm pstl1strm, [x1, #15]
There's also an awkward edge case to check. Binutils supports assembling (for
example)
prfm pstl1strm, [x1, #15]
as if it were
prfum pstl1strm, [x1, #15]
(ordinarily prfm immediate offsets are supposed to be multiples of 8).
Since this is just a programmer-friendly alias for prfum, I think we should reject
prfm ir, [x1, #15].
Can you add a test for this as well?
> 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..01632aaf532
> --- /dev/null
> +++ b/gas/testsuite/gas/aarch64/stshh.s
> @@ -0,0 +1,6 @@
> +// Test file for AArch64 stshh.
> +
> + .text
> +
> + 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..c097e4d01cf 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(), \
> + "STSHH policies 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