public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: binutils@sourceware.org
Subject: Re: [PATCH] RISC-V: fix JAL aliases ordering in opcode table
Date: Thu, 5 Jan 2023 09:20:54 +0100	[thread overview]
Message-ID: <9975504c-579e-48ee-751a-66c6b055a78c@suse.com> (raw)
In-Reply-To: <Y7aClIl4B+co2/rG@aurel32.net>

On 05.01.2023 08:56, Aurelien Jarno wrote:
> On 2023-01-04 08:50, Jan Beulich via Binutils wrote:
>> On 04.01.2023 06:59, Aurelien Jarno wrote:
>>> Commit 839189bc932e ("RISC-V: re-arrange opcode table for consistent
>>> alias handling") reorder the instructions in the opcode tables,
>>> including the various JAL aliases. In particular they are not ordered
>>> anymore from the most specific to the less specific one. This causes the
>>> form "JAL reg, imm" to emit a relocation with the register name. This
>>> breaks various things like building Linux kernel modules.
>>>
>>> This patch fixes the issue by restoring the original ordering of the JAL
>>> aliases.
>>
>> I'm afraid reverting the "offending" hunk is only a workaround, as it
>> would re-introduce the disassembler side issue that the patch tried to
>> deal with (and which sadly looks to not be covered by anything in the
>> testsuite, or else you would have noticed breakage when testing your
>> change). Since I expect that in the course of table lookup it is still
>> the correct entry (in particular one with two rather than just one
>> operands) which is used, my suspicion is that something somewhere makes
>> a silent assumption upon the (original) ordering of table entries. It
>> would be that place which needs fixing then; I'll try to find time soon
>> to see if I can spot where that is.
> 
> Thanks for the analysis. Do you think you have time to get a patch with
> that fix before the 2.40 release?

I hope to get to looking into this tomorrow; whether that'll result in a
patch I obviously can't tell yet. In any event Nelson's reply (which
crossed with mine) suggests the reason for the problem to be different.
Yet then still that's where the change wants to be (albeit I disagree
with his proposal, yet that's without having looked at the code at all),
rather than re-introducing the disassembler inconsistency. The latter
would certainly be a short-term option for the impending release, if a
proper solution can't be found in time or turns out too intrusive. But
then the title and description would need adjustment to reflect this
(e.g. it wouldn't be "fix" but maybe "restore" or "undo").

Jan

  reply	other threads:[~2023-01-05  8:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-04  5:59 Aurelien Jarno
2023-01-04  7:43 ` Nelson Chu
2023-01-04  7:50 ` Jan Beulich
2023-01-05  7:56   ` Aurelien Jarno
2023-01-05  8:20     ` Jan Beulich [this message]
2023-01-08 13:53       ` Maciej W. Rozycki

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=9975504c-579e-48ee-751a-66c6b055a78c@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    /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).