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

[Palmer Dabbelt]

On Thu, 10 Aug 2023 08:31:46 PDT (-0700), ajo...@ventanamicro.com wrote:
On Mon, Jul 31, 2023 at 11:33:20AM -0700, Richard Bagley wrote:
The recent commit 36df75a0a9 corrected one aspect of LUI disassembly
by recovering the immediate argument from the result of LUI with a
shift right by 12. However, the shift right will left-fill with the
sign. By applying a mask we recover an unsigned representation of the
20-bit field (which includes a sign bit).
Example:
0xfffff000 >> 12 = 0xffffffff
0xfffff000 >> 12 & 0xfffff = 0x000fffff

Fixes: 36df75a0a9 ("riscv/disas: Fix disas output of upper immediates")
Signed-off-by: Richard Bagley <rbag...@ventanamicro.com>
---
 disas/riscv.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 4023e3fc65..690eb4a1ac 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -4723,9 +4723,12 @@ static void format_inst(char *buf, size_t buflen, size_t 
tab, rv_decode *dec)
             break;
         case 'U':
             fmt++;
-            snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12);
-            append(buf, tmp, buflen);
-            if (*fmt == 'o') {
+            if (*fmt == 'i') {
+                snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12 & 0xfffff);

Why are we correcting LUI's output, but still outputting sign-extended
values for AUIPC?

We can't assemble 'auipc a1, 0xffffffff' or 'auipc a1, -1' without getting

 Error: lui expression not in range 0..1048575

(and additionally for 0xffffffff)

 Error: value of 00000ffffffff000 too large for field of 4 bytes at 
0000000000000000

either.

(I see that the assembler's error messages state 'lui', but I was trying
'auipc'.)

I'm using as from gnu binutils 2.40.0.20230214.

(And, FWIW, I agree with Richard Henderson that these instructions should
accept negative values.)

I'm kind of lost here, and you saying binutils rejects this syntax?  If 
that's the case it's probably just an oversight, can you file a bug in 
binutils land so folks can see?


Thanks,
drew


+                append(buf, tmp, buflen);
+            } else if (*fmt == 'o') {
+                snprintf(tmp, sizeof(tmp), "%d", dec->imm >> 12);
+                append(buf, tmp, buflen);
                 while (strlen(buf) < tab * 2) {
                     append(buf, " ", buflen);
                 }
--
2.34.1