[PATCH] RISC-V: remove indirection from register tables

[Tsukasa OI]

Not bad :)

On 2023/08/11 22:18, Jan Beulich via Binutils wrote:
> The longest register name is 4 characters (plus a nul one), so using a
> 4- or 8-byte pointer to get at it is neither space nor time efficient.
> Embed the names right into the array. For PIE this also reduces the
> number of base relocations in the final image.
> 
> To avoid old gcc, when generating 32-bit code, bogusly warning about
> bounds being exceeded in the code processing Cs/Cw, Ct/Cx, and CD,
> an adjustment to EXTRACT_BITS() is needed: This macro shouldn't supply
> a 64-bit value, and it also doesn't need to - all operand fields to
> date are far more narrow than 32 bits. This in turn allows dropping a
> number of casts elsewhere.
> ---
> Of course this way the array items aren't a power of 2 in size anymore.
> While adding padding would still keep overall size below the original
> for 64-bit code, in 32-bit code the space savings would likely be lost.
> The alternative of setting NRC to 4 and hence omitting the nul character
> in some of the slots may not be liked, for requiring all use sites to
> then be aware of that aspect and using %.4s instead of plain %s.
> 
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -951,7 +951,7 @@ hash_reg_name (enum reg_class class, con
>  }
>  
>  static void
> -hash_reg_names (enum reg_class class, const char * const names[], unsigned n)
> +hash_reg_names (enum reg_class class, const char names[][NRC], unsigned n)
>  {
>    unsigned i;
>  
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -355,7 +355,7 @@ static inline unsigned int riscv_insn_le
>  
>  /* Extract the operand given by FIELD from integer INSN.  */
>  #define EXTRACT_OPERAND(FIELD, INSN) \
> -  EXTRACT_BITS ((INSN), OP_MASK_##FIELD, OP_SH_##FIELD)
> +  ((unsigned int) EXTRACT_BITS ((INSN), OP_MASK_##FIELD, OP_SH_##FIELD))
>  
>  /* Extract an unsigned immediate operand on position s with n bits.  */
>  #define EXTRACT_U_IMM(n, s, l) \
> @@ -575,14 +575,16 @@ enum riscv_seg_mstate
>    MAP_INSN,		/* Instructions.  */
>  };
>  
> -extern const char * const riscv_gpr_names_numeric[NGPR];
> -extern const char * const riscv_gpr_names_abi[NGPR];
> -extern const char * const riscv_fpr_names_numeric[NFPR];
> -extern const char * const riscv_fpr_names_abi[NFPR];
> +#define NRC 5           /* Max characters in register names, incl nul.  */

I'd prefer (4+1).  Otherwise, it looks okay.

I also would like to hear others about the naming of this constant
"NRC".  It's short and not bad (even if unchanged, it's okay for me)...
but I feel there's a better one exists (even if it's longer).

Thanks,
Tsukasa

> +
> +extern const char riscv_gpr_names_numeric[NGPR][NRC];
> +extern const char riscv_gpr_names_abi[NGPR][NRC];
> +extern const char riscv_fpr_names_numeric[NFPR][NRC];
> +extern const char riscv_fpr_names_abi[NFPR][NRC];
>  extern const char * const riscv_rm[8];
>  extern const char * const riscv_pred_succ[16];
> -extern const char * const riscv_vecr_names_numeric[NVECR];
> -extern const char * const riscv_vecm_names_numeric[NVECM];
> +extern const char riscv_vecr_names_numeric[NVECR][NRC];
> +extern const char riscv_vecm_names_numeric[NVECM][NRC];
>  extern const char * const riscv_vsew[8];
>  extern const char * const riscv_vlmul[8];
>  extern const char * const riscv_vta[2];
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -69,8 +69,8 @@ static enum riscv_seg_mstate last_map_st
>  static asection *last_map_section = NULL;
>  
>  /* Register names as used by the disassembler.  */
> -static const char * const *riscv_gpr_names;
> -static const char * const *riscv_fpr_names;
> +static const char (*riscv_gpr_names)[NRC];
> +static const char (*riscv_fpr_names)[NRC];
>  
>  /* If set, disassemble as most general instruction.  */
>  static bool no_aliases = false;
> @@ -502,7 +502,7 @@ print_insn_args (const char *oparg, insn
>  
>  	case 'y':
>  	  print (info->stream, dis_style_immediate, "0x%x",
> -		 (unsigned)EXTRACT_OPERAND (BS, l));
> +		 EXTRACT_OPERAND (BS, l));
>  	  break;
>  
>  	case 'z':
> @@ -511,12 +511,12 @@ print_insn_args (const char *oparg, insn
>  
>  	case '>':
>  	  print (info->stream, dis_style_immediate, "0x%x",
> -		 (unsigned)EXTRACT_OPERAND (SHAMT, l));
> +		 EXTRACT_OPERAND (SHAMT, l));
>  	  break;
>  
>  	case '<':
>  	  print (info->stream, dis_style_immediate, "0x%x",
> -		 (unsigned)EXTRACT_OPERAND (SHAMTW, l));
> +		 EXTRACT_OPERAND (SHAMTW, l));
>  	  break;
>  
>  	case 'S':
> @@ -577,7 +577,7 @@ print_insn_args (const char *oparg, insn
>  
>  	case 'Y':
>  	  print (info->stream, dis_style_immediate, "0x%x",
> -		 (unsigned) EXTRACT_OPERAND (RNUM, l));
> +		 EXTRACT_OPERAND (RNUM, l));
>  	  break;
>  
>  	case 'Z':
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -26,7 +26,7 @@
>  
>  /* Register names used by gas and objdump.  */
>  
> -const char * const riscv_gpr_names_numeric[NGPR] =
> +const char riscv_gpr_names_numeric[NGPR][NRC] =
>  {
>    "x0",   "x1",   "x2",   "x3",   "x4",   "x5",   "x6",   "x7",
>    "x8",   "x9",   "x10",  "x11",  "x12",  "x13",  "x14",  "x15",
> @@ -34,7 +34,7 @@ const char * const riscv_gpr_names_numer
>    "x24",  "x25",  "x26",  "x27",  "x28",  "x29",  "x30",  "x31"
>  };
>  
> -const char * const riscv_gpr_names_abi[NGPR] =
> +const char riscv_gpr_names_abi[NGPR][NRC] =
>  {
>    "zero", "ra",   "sp",   "gp",   "tp",   "t0",   "t1",   "t2",
>    "s0",   "s1",   "a0",   "a1",   "a2",   "a3",   "a4",   "a5",
> @@ -42,7 +42,7 @@ const char * const riscv_gpr_names_abi[N
>    "s8",   "s9",   "s10",  "s11",  "t3",   "t4",   "t5",   "t6"
>  };
>  
> -const char * const riscv_fpr_names_numeric[NFPR] =
> +const char riscv_fpr_names_numeric[NFPR][NRC] =
>  {
>    "f0",   "f1",   "f2",   "f3",   "f4",   "f5",   "f6",   "f7",
>    "f8",   "f9",   "f10",  "f11",  "f12",  "f13",  "f14",  "f15",
> @@ -50,7 +50,7 @@ const char * const riscv_fpr_names_numer
>    "f24",  "f25",  "f26",  "f27",  "f28",  "f29",  "f30",  "f31"
>  };
>  
> -const char * const riscv_fpr_names_abi[NFPR] =
> +const char riscv_fpr_names_abi[NFPR][NRC] =
>  {
>    "ft0",  "ft1",  "ft2",  "ft3",  "ft4",  "ft5",  "ft6",  "ft7",
>    "fs0",  "fs1",  "fa0",  "fa1",  "fa2",  "fa3",  "fa4",  "fa5",
> @@ -72,7 +72,7 @@ const char * const riscv_pred_succ[16] =
>  };
>  
>  /* RVV registers.  */
> -const char * const riscv_vecr_names_numeric[NVECR] =
> +const char riscv_vecr_names_numeric[NVECR][NRC] =
>  {
>    "v0",   "v1",   "v2",   "v3",   "v4",   "v5",   "v6",   "v7",
>    "v8",   "v9",   "v10",  "v11",  "v12",  "v13",  "v14",  "v15",
> @@ -81,7 +81,7 @@ const char * const riscv_vecr_names_nume
>  };
>  
>  /* RVV mask registers.  */
> -const char * const riscv_vecm_names_numeric[NVECM] =
> +const char riscv_vecm_names_numeric[NVECM][NRC] =
>  {
>    "v0.t"
>  };
>