public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Srinath Parvathaneni <srinath.parvathaneni@arm.com>
To: Jan Beulich <jbeulich@suse.com>, binutils@sourceware.org
Cc: richard.earnshaw@arm.com, nickc@redhat.com,
	Marcus Shawcroft <marcus.shawcroft@arm.com>
Subject: Re: [Committed][PATCH v1 1/1][Binutils] aarch64: Fix the 2nd operand in gcsstr and gcssttr instructions.
Date: Thu, 29 Feb 2024 21:39:12 +0000	[thread overview]
Message-ID: <3061a86a-e94d-4e5f-bf3a-c8210da20781@arm.com> (raw)
In-Reply-To: <d36b052e-1c7c-406f-9d38-14d97375f91e@suse.com>

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(-)
>>

      reply	other threads:[~2024-02-29 21:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-13 18:03 [PATCH " srinath
2024-02-14  9:24 ` Jan Beulich
2024-02-29 21:39   ` Srinath Parvathaneni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3061a86a-e94d-4e5f-bf3a-c8210da20781@arm.com \
    --to=srinath.parvathaneni@arm.com \
    --cc=binutils@sourceware.org \
    --cc=jbeulich@suse.com \
    --cc=marcus.shawcroft@arm.com \
    --cc=nickc@redhat.com \
    --cc=richard.earnshaw@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).