* [PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions.
@ 2024-02-13 18:03 srinath
2024-02-14 9:24 ` Jan Beulich
0 siblings, 1 reply; 3+ messages in thread
From: srinath @ 2024-02-13 18:03 UTC (permalink / raw)
To: binutils; +Cc: richard.earnshaw, nickc, Srinath Parvathaneni
[-- Attachment #1: Type: text/plain, Size: 44 bytes --]
This is a multi-part message in MIME format.
[-- Attachment #2: Type: text/plain, Size: 681 bytes --]
Hi,
The assembler wrongly expects plain register name instead of
memory-form 2nd operand for gcsstr and gcssttr instructions.
This patch fixes the issue.
Regression testing for aarch64-none-elf target and found no regressions.
Ok for binutils-master? Also ok to be backported to binutils-2.42 branch?
Regards,
Srinath.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31376
---
gas/testsuite/gas/aarch64/gcs-1-bad.l | 48 +++++++++++++--------------
gas/testsuite/gas/aarch64/gcs-1.d | 48 +++++++++++++--------------
gas/testsuite/gas/aarch64/gcs-1.s | 2 +-
opcodes/aarch64-tbl.h | 4 +--
4 files changed, 51 insertions(+), 51 deletions(-)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v1-0001-PATCH-Binutils-aarch64-Fix-the-2nd-operand-in-gcs.patch --]
[-- Type: text/x-patch; name="v1-0001-PATCH-Binutils-aarch64-Fix-the-2nd-operand-in-gcs.patch", Size: 7613 bytes --]
diff --git a/gas/testsuite/gas/aarch64/gcs-1-bad.l b/gas/testsuite/gas/aarch64/gcs-1-bad.l
index ca8d17ab8fc..4c69c6e1c57 100644
--- a/gas/testsuite/gas/aarch64/gcs-1-bad.l
+++ b/gas/testsuite/gas/aarch64/gcs-1-bad.l
@@ -19,27 +19,27 @@
[^ :]+:[0-9]+: Error: selected processor does not support `gcspopm x15'
[^ :]+:[0-9]+: Error: selected processor does not support `gcspopm x30'
[^ :]+:[0-9]+: Error: selected processor does not support `gcspopm xzr'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,sp'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,x1'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,x16'
-[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,sp'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x0,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x15,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr x30,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcsstr xzr,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x0,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x15,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr x30,\[sp\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,\[x1\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,\[x16\]'
+[^ :]+:[0-9]+: Error: selected processor does not support `gcssttr xzr,\[sp\]'
diff --git a/gas/testsuite/gas/aarch64/gcs-1.d b/gas/testsuite/gas/aarch64/gcs-1.d
index 09fa418e5ea..ff059a36525 100644
--- a/gas/testsuite/gas/aarch64/gcs-1.d
+++ b/gas/testsuite/gas/aarch64/gcs-1.d
@@ -29,27 +29,27 @@
.*: d52b772f gcspopm x15
.*: d52b773e gcspopm x30
.*: d52b773f gcspopm
-.*: d91f0c20 gcsstr x0, x1
-.*: d91f0e00 gcsstr x0, x16
-.*: d91f0fe0 gcsstr x0, sp
-.*: d91f0c2f gcsstr x15, x1
-.*: d91f0e0f gcsstr x15, x16
-.*: d91f0fef gcsstr x15, sp
-.*: d91f0c3e gcsstr x30, x1
-.*: d91f0e1e gcsstr x30, x16
-.*: d91f0ffe gcsstr x30, sp
-.*: d91f0c3f gcsstr xzr, x1
-.*: d91f0e1f gcsstr xzr, x16
-.*: d91f0fff gcsstr xzr, sp
-.*: d91f1c20 gcssttr x0, x1
-.*: d91f1e00 gcssttr x0, x16
-.*: d91f1fe0 gcssttr x0, sp
-.*: d91f1c2f gcssttr x15, x1
-.*: d91f1e0f gcssttr x15, x16
-.*: d91f1fef gcssttr x15, sp
-.*: d91f1c3e gcssttr x30, x1
-.*: d91f1e1e gcssttr x30, x16
-.*: d91f1ffe gcssttr x30, sp
-.*: d91f1c3f gcssttr xzr, x1
-.*: d91f1e1f gcssttr xzr, x16
-.*: d91f1fff gcssttr xzr, sp
+.*: d91f0c20 gcsstr x0, \[x1\]
+.*: d91f0e00 gcsstr x0, \[x16\]
+.*: d91f0fe0 gcsstr x0, \[sp\]
+.*: d91f0c2f gcsstr x15, \[x1\]
+.*: d91f0e0f gcsstr x15, \[x16\]
+.*: d91f0fef gcsstr x15, \[sp\]
+.*: d91f0c3e gcsstr x30, \[x1\]
+.*: d91f0e1e gcsstr x30, \[x16\]
+.*: d91f0ffe gcsstr x30, \[sp\]
+.*: d91f0c3f gcsstr xzr, \[x1\]
+.*: d91f0e1f gcsstr xzr, \[x16\]
+.*: d91f0fff gcsstr xzr, \[sp\]
+.*: d91f1c20 gcssttr x0, \[x1\]
+.*: d91f1e00 gcssttr x0, \[x16\]
+.*: d91f1fe0 gcssttr x0, \[sp\]
+.*: d91f1c2f gcssttr x15, \[x1\]
+.*: d91f1e0f gcssttr x15, \[x16\]
+.*: d91f1fef gcssttr x15, \[sp\]
+.*: d91f1c3e gcssttr x30, \[x1\]
+.*: d91f1e1e gcssttr x30, \[x16\]
+.*: d91f1ffe gcssttr x30, \[sp\]
+.*: d91f1c3f gcssttr xzr, \[x1\]
+.*: d91f1e1f gcssttr xzr, \[x16\]
+.*: d91f1fff gcssttr xzr, \[sp\]
diff --git a/gas/testsuite/gas/aarch64/gcs-1.s b/gas/testsuite/gas/aarch64/gcs-1.s
index 35584a8810e..17734f9f979 100644
--- a/gas/testsuite/gas/aarch64/gcs-1.s
+++ b/gas/testsuite/gas/aarch64/gcs-1.s
@@ -14,7 +14,7 @@
.irp op gcsstr, gcssttr
.irp reg1 x0, x15, x30, xzr
.irp reg2 x1, x16, sp
- \op \reg1, \reg2
+ \op \reg1, [\reg2]
.endr
.endr
.endr
diff --git a/opcodes/aarch64-tbl.h b/opcodes/aarch64-tbl.h
index 9ea4de01c60..18ac80251e9 100644
--- a/opcodes/aarch64-tbl.h
+++ b/opcodes/aarch64-tbl.h
@@ -4283,8 +4283,8 @@ const struct aarch64_opcode aarch64_opcode_table[] =
GCS_INSN ("gcsss2", 0xd52b7760, 0xffffffe0, OP1 (Rt), QL_I1X, 0),
GCS_INSN ("gcspopm", 0xd52b773f, 0xffffffff, OP0 (), {}, 0),
GCS_INSN ("gcspopm", 0xd52b7720, 0xffffffe0, OP1 (Rt), QL_I1X, 0),
- GCS_INSN ("gcsstr", 0xd91f0c00, 0xfffffc00, OP2 (Rt, Rn_SP), QL_I2SAMEX, 0),
- GCS_INSN ("gcssttr", 0xd91f1c00, 0xfffffc00, OP2 (Rt, Rn_SP), QL_I2SAMEX, 0),
+ GCS_INSN ("gcsstr", 0xd91f0c00, 0xfffffc00, OP2 (Rt, ADDR_SIMPLE), QL_DST_X, 0),
+ GCS_INSN ("gcssttr", 0xd91f1c00, 0xfffffc00, OP2 (Rt, ADDR_SIMPLE), QL_DST_X, 0),
CORE_INSN ("gcsb", 0xd503227f, 0xffffffff, ic_system, 0, OP1 (BARRIER_GCSB), {}, F_ALIAS),
CORE_INSN ("sys", 0xd5080000, 0xfff80000, ic_system, 0, OP5 (UIMM3_OP1, CRn, CRm, UIMM3_OP2, Rt), QL_SYS, F_HAS_ALIAS | F_OPD4_OPT | F_DEFAULT (0x1F)),
D128_INSN ("sysp", 0xd5480000, 0xfff80000, OP6 (UIMM3_OP1, CRn, CRm, UIMM3_OP2, Rt, PAIRREG_OR_XZR), QL_SYSP, F_HAS_ALIAS | F_OPD_NARROW | F_OPD4_OPT | F_OPD_PAIR_OPT | F_DEFAULT (0x1f)),
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions.
2024-02-13 18:03 [PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions srinath
@ 2024-02-14 9:24 ` Jan Beulich
2024-02-29 21:39 ` [Committed][PATCH " Srinath Parvathaneni
0 siblings, 1 reply; 3+ messages in thread
From: Jan Beulich @ 2024-02-14 9:24 UTC (permalink / raw)
To: srinath; +Cc: richard.earnshaw, nickc, binutils, Marcus Shawcroft
On 13.02.2024 19:03, srinath wrote:
> The assembler wrongly expects plain register name instead of
> memory-form 2nd operand for gcsstr and gcssttr instructions.
> This patch fixes the issue.
>
> Regression testing for aarch64-none-elf target and found no regressions.
>
> Ok for binutils-master? Also ok to be backported to binutils-2.42 branch?
Looks good to me, but I'm not an Arm64 maintainer. I'd prefer if one of
the two (you didn't even Cc Marcus) would ack the patch, but in case you
don't hear back within a week, feel free to put in on both branches.
That said, ...
> ---
> gas/testsuite/gas/aarch64/gcs-1-bad.l | 48 +++++++++++++--------------
... the need for you to alter the expectations here indicates that the
testcase itself likely was having too specific expectations. For the
intended purpose, the exact set of operands isn't of interest and hence
would likely better have been omitted in the first place. I'm aware
though that it's a common approach to simply not edit expectations from
obtained tool output any further than to add the necessary escaping.
Yet I'm trying to encourage people to put in a little more effort, in
an attempt to limit follow-on effort for themselves or others.
The more thoroughly one zaps irrelevant parts from expectations, the
more likely it also is that one might spot actual issues - this isn't
the case here, i.e. I'm merely trying to provide some further
background, but I've seen it numerous times that expectations were
derived largely blindly from tool output, without checking that the
output is actually correct/sensible in the first place. (I'm afraid
I, too, have been guilty of that in a few cases.)
Jan
> gas/testsuite/gas/aarch64/gcs-1.d | 48 +++++++++++++--------------
> gas/testsuite/gas/aarch64/gcs-1.s | 2 +-
> opcodes/aarch64-tbl.h | 4 +--
> 4 files changed, 51 insertions(+), 51 deletions(-)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Committed][PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions.
2024-02-14 9:24 ` Jan Beulich
@ 2024-02-29 21:39 ` Srinath Parvathaneni
0 siblings, 0 replies; 3+ messages in thread
From: Srinath Parvathaneni @ 2024-02-29 21:39 UTC (permalink / raw)
To: Jan Beulich, binutils; +Cc: richard.earnshaw, nickc, Marcus Shawcroft
Hi
On 2/14/2024 9:24 AM, Jan Beulich wrote:
> On 13.02.2024 19:03, srinath wrote:
>> The assembler wrongly expects plain register name instead of
>> memory-form 2nd operand for gcsstr and gcssttr instructions.
>> This patch fixes the issue.
>>
>> Regression testing for aarch64-none-elf target and found no regressions.
>>
>> Ok for binutils-master? Also ok to be backported to binutils-2.42 branch?
> Looks good to me, but I'm not an Arm64 maintainer. I'd prefer if one of
> the two (you didn't even Cc Marcus) would ack the patch, but in case you
> don't hear back within a week, feel free to put in on both branches.
> That said, ...
I have committed the patch to both "master" and "binutils-2_42-branch".
>> ---
>> gas/testsuite/gas/aarch64/gcs-1-bad.l | 48 +++++++++++++--------------
> ... the need for you to alter the expectations here indicates that the
> testcase itself likely was having too specific expectations. For the
> intended purpose, the exact set of operands isn't of interest and hence
> would likely better have been omitted in the first place. I'm aware
> though that it's a common approach to simply not edit expectations from
> obtained tool output any further than to add the necessary escaping.
> Yet I'm trying to encourage people to put in a little more effort, in
> an attempt to limit follow-on effort for themselves or others.
>
> The more thoroughly one zaps irrelevant parts from expectations, the
> more likely it also is that one might spot actual issues - this isn't
> the case here, i.e. I'm merely trying to provide some further
> background, but I've seen it numerous times that expectations were
> derived largely blindly from tool output, without checking that the
> output is actually correct/sensible in the first place. (I'm afraid
> I, too, have been guilty of that in a few cases.)
>
> Jan
Thanks for the feedback, in my next set of patches (bug fixes to sve2p1)
I shall explain
the logic I'm using to write the tests (covering most of the encoding).
Also, I'm comparing
the outcome of the disassembler with other tools, to avoid these kind of
errors.
Regards,
Srinath.
>> gas/testsuite/gas/aarch64/gcs-1.d | 48 +++++++++++++--------------
>> gas/testsuite/gas/aarch64/gcs-1.s | 2 +-
>> opcodes/aarch64-tbl.h | 4 +--
>> 4 files changed, 51 insertions(+), 51 deletions(-)
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-29 21:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 18:03 [PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions srinath
2024-02-14 9:24 ` Jan Beulich
2024-02-29 21:39 ` [Committed][PATCH " Srinath Parvathaneni
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).