qemu-riscv.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Jones <ajones@ventanamicro.com>
To: Richard Bagley <rbagley@ventanamicro.com>
Cc: Palmer Dabbelt <palmer@rivosinc.com>,
	qemu-devel@nongnu.org,  qemu-riscv@nongnu.org,
	Alistair Francis <Alistair.Francis@wdc.com>,
	bmeng@tinylab.org,  liweiwei@iscas.ac.cn,
	zhiwei_liu@linux.alibaba.com, dbarboza@ventanamicro.com
Subject: Re: [PATCH] disas/riscv: Further correction to LUI disassembly
Date: Tue, 12 Mar 2024 12:09:46 +0100	[thread overview]
Message-ID: <20240312-ed806a55958fd3526ec0f01a@orel> (raw)
In-Reply-To: <CAARkPA_8zSZK-4oOTcJO2KinWSadxZYZztwRP-UvNNkDjmyexw@mail.gmail.com>

On Mon, Mar 11, 2024 at 11:56:42AM -0700, Richard Bagley wrote:
> I have realized that *the patch is indeed a fix*, not a workaround.
> 
> In fact, the argument to LUI and AUIPC in assembly *must* be a number
> between [0x0, 0xfffff].
> RISC-V Assembly Programmer's Manual : Load Upper Immediate's Immediate
> <https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#load-upper-immediates-immediate>

I think that's just documenting the current behavior, but the behavior
(not accepting a signed decimal number for a signed immediate) doesn't
appear to be justified, so I think my suggestion in [1] still stands.
That said, I don't really have much of a horse in this race so if
somebody comes along and closes that BZ with a simple justification of
"we, the people that work on this stuff, agreed we prefer the range
[0x0, 0xfffff]", then I won't argue.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=30746

Thanks,
drew

> Signed decimal numbers are programmed as their two's complement.
> 
> I checked: neither GCC nor LLVM will assemble
> 
> > lui x1, -4
> 
> The LLVM compiled models the arguments to LUI and AUIPC as UIMM (unsigned
> immediate) or UIMM20 (20 bit unsigned immediate).
> 
> I should have checked this from the start. I jumped to the conclusion that
> both formats (signed decimal, two's complement) for negative arguments
> should be supported, and that I was encountering a bug.
> I apologize to all for the unnecessary back-and-forth.
> 
> I don't yet see a reason why llvm and gcc could not support a signed number
> in decimal format, perhaps requiring a pseudo-instruction.
> This might be desirable, if only in support of assembly programming.
> On the other hand, it is easy to make the conversion to a two's-complement
> number.
> 
> Richard
> 
> On Sat, Mar 9, 2024 at 4:01 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> 
> > On Fri, Mar 08, 2024 at 08:22:01PM -0800, Richard Bagley wrote:
> > > post-nack, one further comment:
> > >
> > > One could argue that this change also aligns QEMU with supporting tools
> > (as
> > > Andrew observed), and it makes sense to merge this change into QEMU until
> > > those tools update to supporting signed decimal numbers with immediates.
> > >
> > > As it is, both GNU assembler and the LLVM integrated assembler (or
> > llvm-mc)
> > > throws an error with examples such as
> > > auipc s0, -17
> > >
> > > On the other hand, I have only seen this problem with the output of the
> > > COLLECT plug-in, not (as yet) with QEMU execution proper.
> > > If the problem is confined to COLLECT, perhaps the argument for aligning
> > > with other tools is not as strong.
> > >
> > > In the meantime, I have adjusted my change locally to include AUIPC, and
> > > written a substantive, and I hope, clear commit description.
> > > If you would like me to resubmit a patch with this updated change, please
> > > let me know.
> >
> > Since the patch is ready for posting, then it might as well be posted
> > (even if it may not get merged right away). If the issue arises again,
> > then we can refer to the latest proposed patch, which will be preserved
> > in the mail archives.
> >
> > Thanks,
> > drew
> >


      parent reply	other threads:[~2024-03-12 11:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-31 18:33 [PATCH] disas/riscv: Further correction to LUI disassembly Richard Bagley
2023-07-31 20:37 ` Richard Henderson
2023-08-07 22:01   ` Richard Bagley
2023-08-07 22:59     ` Richard Henderson
2023-08-10 15:31 ` Andrew Jones
2023-08-10 16:12   ` Palmer Dabbelt
2023-08-10 16:27     ` Andrew Jones
2023-08-11  8:25       ` Andrew Jones
2023-08-11 11:55         ` Andrew Jones
2024-03-08  0:08           ` Richard Bagley
2024-03-09  4:22             ` Richard Bagley
2024-03-09 12:01               ` Andrew Jones
2024-03-11 18:56                 ` Richard Bagley
2024-03-11 20:00                   ` Richard Bagley
2024-03-12 11:09                   ` Andrew Jones [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=20240312-ed806a55958fd3526ec0f01a@orel \
    --to=ajones@ventanamicro.com \
    --cc=Alistair.Francis@wdc.com \
    --cc=bmeng@tinylab.org \
    --cc=dbarboza@ventanamicro.com \
    --cc=liweiwei@iscas.ac.cn \
    --cc=palmer@rivosinc.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=rbagley@ventanamicro.com \
    --cc=zhiwei_liu@linux.alibaba.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).