Thanks, looks good. I don't have any strong naming opinions here, so it's up to the author of patch :-) Nelson On Fri, Aug 11, 2023 at 9:18 PM Jan Beulich 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. */ > + > +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" > }; >