Re: [PATCH] RISC-V: fix JAL aliases ordering in opcode table

[Nelson Chu <nelson@rivosinc.com>]

This solution is quite simple and clear, so it looks good to me.  The
other solution is that - check for the integer register at the start
of the my_getExpression, just like what my_getSmallExpression did
before.  Although my_getSmallExpression will also call
my_getExpression, so checking the integer register twice looks
redundant, but at least it is more safe and less error-prone.  Or we
can find a way to only check the register once, and make the source
code clear.  But since the release is close, I suggest we just accept
this simple solution in the short-term.

Thanks
Nelson

On Wed, Jan 4, 2023 at 2:00 PM Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> Commit 839189bc932e ("RISC-V: re-arrange opcode table for consistent
> alias handling") reorder the instructions in the opcode tables,
> including the various JAL aliases. In particular they are not ordered
> anymore from the most specific to the less specific one. This causes the
> form "JAL reg, imm" to emit a relocation with the register name. This
> breaks various things like building Linux kernel modules.
>
> This patch fixes the issue by restoring the original ordering of the JAL
> aliases.
>
> Tested on riscv64-linux
> PR gas/29940
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29940
> ---
>  opcodes/riscv-opc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 6b65296a3f2..89c6572519e 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -340,9 +340,9 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"jalr",        0, INSN_CLASS_I, "d,s,j",     MATCH_JALR, MASK_JALR, match_opcode, INSN_JSR },
>  {"j",           0, INSN_CLASS_C, "Ca",        MATCH_C_J, MASK_C_J, match_opcode, INSN_ALIAS|INSN_BRANCH },
>  {"j",           0, INSN_CLASS_I, "a",         MATCH_JAL, MASK_JAL|MASK_RD, match_opcode, INSN_ALIAS|INSN_BRANCH },
> -{"jal",         0, INSN_CLASS_I, "a",         MATCH_JAL|(X_RA << OP_SH_RD), MASK_JAL|MASK_RD, match_opcode, INSN_ALIAS|INSN_JSR },
>  {"jal",         0, INSN_CLASS_I, "d,a",       MATCH_JAL, MASK_JAL, match_opcode, INSN_JSR },
>  {"jal",        32, INSN_CLASS_C, "Ca",        MATCH_C_JAL, MASK_C_JAL, match_opcode, INSN_ALIAS|INSN_JSR },
> +{"jal",         0, INSN_CLASS_I, "a",         MATCH_JAL|(X_RA << OP_SH_RD), MASK_JAL|MASK_RD, match_opcode, INSN_ALIAS|INSN_JSR },
>  {"call",        0, INSN_CLASS_I, "d,c",       (X_T1 << OP_SH_RS1), (int) M_CALL, match_never, INSN_MACRO },
>  {"call",        0, INSN_CLASS_I, "c",         (X_RA << OP_SH_RS1)|(X_RA << OP_SH_RD), (int) M_CALL, match_never, INSN_MACRO },
>  {"tail",        0, INSN_CLASS_I, "c",         (X_T1 << OP_SH_RS1), (int) M_CALL, match_never, INSN_MACRO },
> --
> 2.35.1
>