[PATCH] aarch64: simplify RCPC3 unpredictable logic
Andrew Carlotti
andrew.carlotti@arm.com
Thu Feb 27 15:16:21 GMT 2025
On Tue, Feb 25, 2025 at 12:03:41PM +0100, Jan Beulich wrote:
> The original observation was that STILP is warned about when everything
> is fine. Documentation, not just for STILP, says explicitly that
> behavior is identical to respective pre-existing insns (for STILP in
> particular that's STP). With that it's unclear why distinct logic was
> added: Other code can be re-used, simply distinguishing by the number of
> operands. This was diagnostics also end up more consistent.
Thanks - I agree that the reuse is better. I wonder whether it would have been
possible to reuse existing instruction classes, but the simpler change you've
made seems good for now.
> Along with adjusting some of the STILP uses in the (positive) testcase,
> also adjust a few STLR to similarly demonstrate that the register
> overlap goes without warning when there's no write-back.
Can you add these as new entries (within the existing file), rather than
changing existing entries? That would ensure we retain all the existing
checks, and would make it clear that the original tests here are still valid.
> ---
> See https://sourceware.org/pipermail/binutils/2024-August/136404.html.
>
> Note also how "load pair transfer" was quite odd in the diagnostic for
> STILP being a store, not a load.
>
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -8465,6 +8465,7 @@ warn_unpredictable_ldst (aarch64_instruc
> case ldst_imm10:
> case ldst_unscaled:
> case ldst_unpriv:
> + ldst_single:
> /* Loading/storing the base register is unpredictable if writeback. */
> if ((aarch64_get_operand_class (opnds[0].type)
> == AARCH64_OPND_CLASS_INT_REG)
> @@ -8477,43 +8478,9 @@ warn_unpredictable_ldst (aarch64_instruc
> break;
>
> case rcpc3:
> - {
> - const int nb_operands = aarch64_num_of_operands (opcode);
> - if (aarch64_get_operand_class (opnds[0].type)
> - == AARCH64_OPND_CLASS_INT_REG)
> - {
> - /* Load Pair transfer with register overlap. */
> - if (nb_operands == 3 && opnds[0].reg.regno == opnds[1].reg.regno)
> - { // ldiapp, stilp
> - as_warn (_("unpredictable load pair transfer with register "
> - "overlap -- `%s'"),
> - str);
> - }
> - /* Loading/storing the base register is unpredictable if writeback. */
> - else if ((nb_operands == 2
> - && opnds[0].reg.regno == opnds[1].addr.base_regno
> - && opnds[1].addr.base_regno != REG_SP
> - && opnds[1].addr.writeback)
> - || (nb_operands == 3
> - && (opnds[0].reg.regno == opnds[2].addr.base_regno
> - || opnds[1].reg.regno == opnds[2].addr.base_regno)
> - && opnds[2].addr.base_regno != REG_SP
> - && opnds[2].addr.writeback))
> - {
> - if (strcmp (opcode->name, "ldapr") == 0
> - || strcmp (opcode->name, "ldiapp") == 0)
> - as_warn (
> - _("unpredictable transfer with writeback (load) -- `%s'"),
> - str);
> - else // stlr, stilp
> - as_warn (
> - _("unpredictable transfer with writeback (store) -- `%s'"),
> - str);
> - }
> - }
> - }
> - break;
> -
> + if (aarch64_num_of_operands (opcode) == 2)
> + goto ldst_single;
> + /* Fall through. */
> case ldstpair_off:
> case ldstnapair_offs:
> case ldstpair_indexed:
> --- a/gas/testsuite/gas/aarch64/rcpc3.d
> +++ b/gas/testsuite/gas/aarch64/rcpc3.d
> @@ -26,10 +26,10 @@ Disassembly of section \.text:
> [^:]+: 99011860 stilp w0, w1, \[x3\]
> [^:]+: d9010860 stilp x0, x1, \[x3, #-16\]!
> [^:]+: 99010860 stilp w0, w1, \[x3, #-8\]!
> -[^:]+: d9011820 stilp x0, x1, \[x1\]
> -[^:]+: d9011800 stilp x0, x1, \[x0\]
> -[^:]+: 99011820 stilp w0, w1, \[x1\]
> -[^:]+: 99011800 stilp w0, w1, \[x0\]
> +[^:]+: d9001800 stilp x0, x0, \[x0\]
> +[^:]+: d9011821 stilp x1, x1, \[x1\]
> +[^:]+: 99001800 stilp w0, w0, \[x0\]
> +[^:]+: 99011821 stilp w1, w1, \[x1\]
> [^:]+: b8bfc020 ldapr w0, \[x1\]
> [^:]+: b8bfc020 ldapr w0, \[x1\]
> [^:]+: f8bfc020 ldapr x0, \[x1\]
> @@ -42,10 +42,10 @@ Disassembly of section \.text:
> [^:]+: d9c0083f ldapr xzr, \[x1\], #8
> [^:]+: 99c00be0 ldapr w0, \[sp\], #4
> [^:]+: d9c00be0 ldapr x0, \[sp\], #8
> -[^:]+: 889ffc20 stlr w0, \[x1\]
> -[^:]+: 889ffc20 stlr w0, \[x1\]
> -[^:]+: c89ffc20 stlr x0, \[x1\]
> -[^:]+: c89ffc20 stlr x0, \[x1\]
> +[^:]+: 889ffc00 stlr w0, \[x0\]
> +[^:]+: 889ffc21 stlr w1, \[x1\]
> +[^:]+: c89ffc00 stlr x0, \[x0\]
> +[^:]+: c89ffc21 stlr x1, \[x1\]
> [^:]+: 99800841 stlr w1, \[x2, #-4\]!
> [^:]+: d9800841 stlr x1, \[x2, #-8\]!
> [^:]+: d980081e stlr x30, \[x0, #-8\]!
> --- a/gas/testsuite/gas/aarch64/rcpc3.s
> +++ b/gas/testsuite/gas/aarch64/rcpc3.s
> @@ -27,10 +27,10 @@
> // destination registers, but the doc mentions that, in the case where no
> // offset is specified, writeback is disabled, and so the writeback overlap
> // for store is fine.
> - stilp x0, x1, [x1]
> - stilp x0, x1, [x0]
> - stilp w0, w1, [x1]
> - stilp w0, w1, [x0]
> + stilp x0, x0, [x0]
> + stilp x1, x1, [x1]
> + stilp w0, w0, [x0]
> + stilp w1, w1, [x1]
>
> ldapr w0, [x1]
> ldapr w0, [x1, #0]
> @@ -45,10 +45,10 @@
> ldapr w0, [sp], #4
> ldapr x0, [sp], #8
>
> - stlr w0, [x1]
> - stlr w0, [x1, #0]
> - stlr x0, [x1]
> - stlr x0, [x1, #0]
> + stlr w0, [x0]
> + stlr w1, [x1, #0]
> + stlr x0, [x0]
> + stlr x1, [x1, #0]
> stlr w1, [x2, #-4]!
> stlr x1, [x2, #-8]!
> stlr x30, [x0, #-8]!
> --- a/gas/testsuite/gas/aarch64/rcpc3-fail.l
> +++ b/gas/testsuite/gas/aarch64/rcpc3-fail.l
> @@ -51,27 +51,23 @@
> .*: Error: invalid increment amount at operand 2 -- `stlr x0,\[x1,#-4\]!'
> .*: Error: invalid increment amount at operand 2 -- `stlr w0,\[x1,#4\]!'
> .*: Error: invalid increment amount at operand 2 -- `stlr x0,\[x1,#8\]!'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `ldiapp w0,w0,\[x1\]'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `ldiapp x0,x0,\[x1\]'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `ldiapp w0,w0,\[x1\],#8'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `ldiapp x0,x0,\[x1\],#16'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `stilp w0,w0,\[x1\]'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `stilp x0,x0,\[x1\]'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `stilp w0,w0,\[x1,#-8\]!'
> -.*: Warning: unpredictable load pair transfer with register overlap -- `stilp x0,x0,\[x1,#-16\]!'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldiapp x0,x1,\[x0\],#16'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldiapp x0,x1,\[x1\],#16'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldiapp w0,w1,\[x0\],#8'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldiapp w0,w1,\[x1\],#8'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldapr x0,\[x0\],#8'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldapr w0,\[x0\],#4'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldapr x1,\[x1\],#8'
> -.*: Warning: unpredictable transfer with writeback \(load\) -- `ldapr x30,\[x30\],#8'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stilp x0,x1,\[x1,#-16\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stilp w0,w1,\[x1,#-8\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stilp x0,x1,\[x0,#-16\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stilp w0,w1,\[x0,#-8\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stlr x0,\[x0,#-8\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stlr w0,\[x0,#-4\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stlr x1,\[x1,#-8\]!'
> -.*: Warning: unpredictable transfer with writeback \(store\) -- `stlr x30,\[x30,#-8\]!'
> \ No newline at end of file
> +.*: Warning: unpredictable load of register pair -- `ldiapp w0,w0,\[x1\]'
> +.*: Warning: unpredictable load of register pair -- `ldiapp x0,x0,\[x1\]'
> +.*: Warning: unpredictable load of register pair -- `ldiapp w0,w0,\[x1\],#8'
> +.*: Warning: unpredictable load of register pair -- `ldiapp x0,x0,\[x1\],#16'
> +.*: Warning: unpredictable transfer with writeback -- `ldiapp x0,x1,\[x0\],#16'
> +.*: Warning: unpredictable transfer with writeback -- `ldiapp x0,x1,\[x1\],#16'
> +.*: Warning: unpredictable transfer with writeback -- `ldiapp w0,w1,\[x0\],#8'
> +.*: Warning: unpredictable transfer with writeback -- `ldiapp w0,w1,\[x1\],#8'
> +.*: Warning: unpredictable transfer with writeback -- `ldapr x0,\[x0\],#8'
> +.*: Warning: unpredictable transfer with writeback -- `ldapr w0,\[x0\],#4'
> +.*: Warning: unpredictable transfer with writeback -- `ldapr x1,\[x1\],#8'
> +.*: Warning: unpredictable transfer with writeback -- `ldapr x30,\[x30\],#8'
> +.*: Warning: unpredictable transfer with writeback -- `stilp x0,x1,\[x1,#-16\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stilp w0,w1,\[x1,#-8\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stilp x0,x1,\[x0,#-16\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stilp w0,w1,\[x0,#-8\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stlr x0,\[x0,#-8\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stlr w0,\[x0,#-4\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stlr x1,\[x1,#-8\]!'
> +.*: Warning: unpredictable transfer with writeback -- `stlr x30,\[x30,#-8\]!'
> --- a/gas/testsuite/gas/aarch64/rcpc3-fail.s
> +++ b/gas/testsuite/gas/aarch64/rcpc3-fail.s
> @@ -81,11 +81,6 @@
> ldiapp w0, w0, [x1], #8
> ldiapp x0, x0, [x1], #16
>
> - stilp w0, w0, [x1]
> - stilp x0, x0, [x1]
> - stilp w0, w0, [x1, #-8]!
> - stilp x0, x0, [x1, #-16]!
> -
> /* Invalid write back overlap (load)*/
> ldiapp x0, x1, [x0], #16
> ldiapp x0, x1, [x1], #16
More information about the Binutils
mailing list