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. Thanks, drew > > > > > 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 > > > > > >