On Thu, Aug 10, 2023 at 06:27:50PM +0200, Andrew Jones wrote: > On Thu, Aug 10, 2023 at 09:12:42AM -0700, Palmer Dabbelt wrote: > > 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? > > Will do. >
https://sourceware.org/bugzilla/show_bug.cgi?id=30746 Thanks, drew