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

[Andrew Jones]

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 <rbagley@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.)

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