Re: [PATCH] disas/riscv: Further correction to LUI disassembly

[Richard Bagley <rbagley@ventanamicro.com>]

[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 3585 bytes --]