This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [Patch][AArch64] - Error on load pair to same register
- From: Ryan Mansfield <rmansfield at qnx dot com>
- To: Richard Earnshaw <rearnsha at arm dot com>, Jiong Wang <Jiong dot Wang at arm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 18 Nov 2014 09:01:32 -0500
- Subject: Re: [Patch][AArch64] - Error on load pair to same register
- Authentication-results: sourceware.org; auth=none
- References: <5463BE02 dot 4000109 at qnx dot com> <546485FB dot 1030600 at arm dot com> <546A0CEC dot 3050205 at qnx dot com> <546B1C0D dot 4050204 at arm dot com>
On 14-11-18 05:14 AM, Richard Earnshaw wrote:
On 17/11/14 14:57, Ryan Mansfield wrote:
2014-11-17 Ryan Mansfield <rmansfield@qnx.com>
* config/tc-aarch64.c (md_assemble): Call warn_unpredictable.
(warn_unpredictable): New.
2014-11-17 Ryan Mansfield <rmansfield@qnx.com>
* gas/aarch64/diagnostic.s: Add new warnings test patterns.
* gas/aarch64/diagnostic.l: Update expected diagnostic output.
Ryan,
Thanks for the patch.
While I can see merit in handling common cases like loads/stores in one
function, I'm not entirely convinced that having one common function to
handle all unpredictable insns is a good idea - the function could get
very big over time with no real gain to be had from the single
interface. As such, I'd rename the function to
warn_unpredictable_ldst() or something like that.
OK with that change.
I've updated the function name, and the comment to be specific for
load/stores. If this is OK, would someone please apply the patch on my
behalf? I don't have write privs.
Here are the updated changelogs and patch:
2014-11-18 Ryan Mansfield <rmansfield@qnx.com>
* config/tc-aarch64.c (md_assemble): Call warn_unpredictable_ldst.
(warn_unpredictable_ldst): New.
2014-11-18 Ryan Mansfield <rmansfield@qnx.com>
* gas/aarch64/diagnostic.s: Add new warnings test patterns.
* gas/aarch64/diagnostic.l: Update expected diagnostic output.
Regards,
Ryan Mansfield
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 8cecfd0..41378f5 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -5490,6 +5490,40 @@ programmer_friendly_fixup (aarch64_instruction *instr)
return TRUE;
}
+/* Check for loads and stores that will cause unpredictable behavior */
+
+static void
+warn_unpredictable_ldst (aarch64_instruction *instr, char *str)
+{
+ aarch64_inst *base = &instr->base;
+ const aarch64_opcode *opcode = base->opcode;
+ const aarch64_opnd_info *opnds = base->operands;
+ switch (opcode->iclass)
+ {
+ case ldst_pos:
+ case ldst_imm9:
+ case ldst_unscaled:
+ case ldst_unpriv:
+ if (opnds[0].reg.regno == opnds[1].reg.regno
+ && opnds[1].addr.writeback)
+ as_warn (_("unpredictable register after writeback -- `%s'"), str);
+ break;
+ case ldstpair_off:
+ case ldstnapair_offs:
+ case ldstpair_indexed:
+ if ((opnds[0].reg.regno == opnds[2].reg.regno
+ || opnds[1].reg.regno == opnds[2].reg.regno)
+ && opnds[2].addr.writeback)
+ as_warn (_("unpredictable register after writeback -- `%s'"), str);
+ if ((opcode->opcode & (1 << 22))
+ && opnds[0].reg.regno == opnds[1].reg.regno)
+ as_warn (_("unpredictable load of register pair -- `%s'"), str);
+ break;
+ default:
+ break;
+ }
+}
+
/* A wrapper function to interface with libopcodes on encoding and
record the error message if there is any.
@@ -5622,6 +5656,8 @@ md_assemble (char *str)
return;
}
+ warn_unpredictable_ldst (&inst, str);
+
if (inst.reloc.type == BFD_RELOC_UNUSED
|| !inst.reloc.need_libopcodes_p)
output_inst (NULL);
diff --git a/gas/testsuite/gas/aarch64/diagnostic.l b/gas/testsuite/gas/aarch64/diagnostic.l
index 322e3c0..d5fdba8 100644
--- a/gas/testsuite/gas/aarch64/diagnostic.l
+++ b/gas/testsuite/gas/aarch64/diagnostic.l
@@ -105,3 +105,12 @@
[^:]*:109: Error: operand 5 should be an integer register -- `sys #0,c0,c0,#0,kk'
[^:]*:110: Error: unexpected comma before the omitted optional operand at operand 5 -- `sys #0,c0,c0,#0,'
[^:]*:112: Error: selected processor does not support `casp w0,w1,w2,w3,\[x4\]'
+[^:]*:115: Warning: unpredictable load of register pair -- `ldp x0,x0,\[sp\]'
+[^:]*:116: Warning: unpredictable load of register pair -- `ldp d0,d0,\[sp\]'
+[^:]*:117: Warning: unpredictable load of register pair -- `ldp x0,x0,\[sp\],#16'
+[^:]*:118: Warning: unpredictable load of register pair -- `ldnp x0,x0,\[sp\]'
+[^:]*:121: Warning: unpredictable register after writeback -- `ldr x0,\[x0,#8\]!'
+[^:]*:122: Warning: unpredictable register after writeback -- `str x0,\[x0,#8\]!'
+[^:]*:123: Warning: unpredictable register after writeback -- `str x1,\[x1\],#8'
+[^:]*:124: Warning: unpredictable register after writeback -- `stp x0,x1,\[x0,#16\]!'
+[^:]*:125: Warning: unpredictable register after writeback -- `ldp x0,x1,\[x1\],#16'
diff --git a/gas/testsuite/gas/aarch64/diagnostic.s b/gas/testsuite/gas/aarch64/diagnostic.s
index b82ac8b..88001da 100644
--- a/gas/testsuite/gas/aarch64/diagnostic.s
+++ b/gas/testsuite/gas/aarch64/diagnostic.s
@@ -110,3 +110,16 @@
sys #0, c0, c0, #0,
casp w0,w1,w2,w3,[x4]
+
+ # test warning of unpredictable load pairs
+ ldp x0, x0, [sp]
+ ldp d0, d0, [sp]
+ ldp x0, x0, [sp], #16
+ ldnp x0, x0, [sp]
+
+ # test warning of unpredictable writeback
+ ldr x0, [x0, #8]!
+ str x0, [x0, #8]!
+ str x1, [x1], #8
+ stp x0, x1, [x0, #16]!
+ ldp x0, x1, [x1], #16