Re: [PATCH v3 1/2] RISC-V: Support Zcmp push/pop instructions.

[Kito Cheng <kito.cheng@gmail.com>]

> diff --git a/bfd/elfxx-riscv.h b/bfd/elfxx-riscv.h
> index abcb409bd78..6f551bb6907 100644
> --- a/bfd/elfxx-riscv.h
> +++ b/bfd/elfxx-riscv.h
> @@ -26,6 +26,7 @@
>  #include "cpu-riscv.h"
>
>  #define RISCV_UNKNOWN_VERSION -1
> +#define SP_ALIGNMENT 16

Rename to ZCMP_SP_ALIGNMENT, since ilp32e may not align to 16, and
this macro may cause confusion .

>
>  struct riscv_elf_params
>  {
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 402c46ad753..a46de00fc32 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -1246,6 +1246,125 @@ flt_lookup (float f, const float *array, size_t size, unsigned *regnop)
>    return false;
>  }
>
> +/* Map ra and s-register to [4,15], so that we can check if the
> +   reg2 in register list reg1-reg2 or single reg2 is valid or not,
> +   and obtain the corresponding rlist value.

rlist -> reg_list

> +
> +   ra - 4
> +   s0 - 5
> +   s1 - 6
> +    ....
> +   s10 - 0 (invalid)
> +   s11 - 15.  */
> +
> +static int
> +regno_to_rlist (unsigned regno)

rlist -> reg_list

> +{
> +  if (regno == X_RA)
> +    return 4;
> +  else if (regno == X_S0 || regno == X_S1)
> +    return 5 + regno - X_S0;
> +  else if (regno >= X_S2 && regno < X_S10)
> +    return 7 + regno - X_S2;
> +  else if (regno == X_S11)
> +    return 15;
> +
> +  return 0; /* invalid symbol */
> +}
> +
> +/* Parse register list, and the parsed rlist value is stored in rlist
> +   argument.
> +
> +   If ABI register names are used (e.g. ra and s0), the register
> +   list could be "{ra}", "{ra, s0}", "{ra, s0-sN}", where 0 < N < 10 or
> +   N == 11.
> +
> +   If numeric register names are used (e.g. x1 and x8), the register list
> +   could be "{x1}", "{x1,x8}", "{x1,x8-x9}", "{x1,x8-x9,x18}" and
> +   "{x1,x8-x9,x18-xN}", where 19 < N < 25 or N == 27.
> +
> +   It will fail if numeric register names and ABI register names are used
> +   at the same time.
> +*/
> +
> +static bool
> +reglist_lookup (char **s, unsigned *rlist)

rlist -> reg_list

> +{
> +  unsigned regno = 0;
> +  unsigned regnum = 0;
> +  char *reglist = strdup(*s);

space before (, e.g. strdup (*s);

> +  char *regname[3];
> +
> +  if (reglist == NULL)
> +    return false;
> +
> +  reglist = strtok(reglist, "}");

space before (

> +  for(reglist = strtok(reglist, ",");reglist;reglist = strtok(NULL, ",")){

space before (

> +    regname[regnum] = reglist;
> +    regnum++;
> +  }
> +
> +  /* Use to check if the register format is xreg.  */
> +  bool use_xreg = **s == 'x';
> +
> +  /* The first register in register list should be ra.  */

in the register list

> +  if (!reg_lookup (s, RCLASS_GPR, &regno)
> +     || !(*rlist = regno_to_rlist (regno)) /* update rlist */
> +     || regno != X_RA)
> +    return false;
> +
> +  if (regnum == 1)
> +    return true;
> +
> +  /* Do not use numeric and abi names at the same time.  */
> +  if ((*++*s != 'x') && use_xreg)
> +    return false;
> +  /* Reg1 should be s0 or its numeric names x8.  */
> +  if (!reg_lookup (s, RCLASS_GPR, &regno)
> +     || !(*rlist = regno_to_rlist (regno))
> +     || regno != X_S0)
> +    return false;
> +
> +  if(strlen(regname[1]) == 2)

space before (

> +    return true;
> +
> +  if ((*++*s != 'x') && use_xreg)
> +    return false;
> +  /* Reg2 is x9 if the numeric name is used, otherwise,
> +    it could be any other sN register, where N > 0.  */
> +  if (!reg_lookup (s, RCLASS_GPR, &regno)
> +     || !(*rlist = regno_to_rlist (regno))
> +     || regno <= X_S0
> +     || (use_xreg && regno != X_S1))
> +    return false;
> +
> +  if (regnum == 2)
> +    return true;
> +
> +  if(regnum == 3 && use_xreg){

space before ( and space between ) {

 if (regnum == 3 && use_xreg) {

> +    if ((*++*s != 'x') && use_xreg)
> +      return false;
> +    /* Reg3 should be s2.  */
> +    if (!reg_lookup (s, RCLASS_GPR, &regno)
> +       || !(*rlist = regno_to_rlist (regno))
> +       || regno != X_S2)
> +      return false;
> +    if(strlen(regname[2]) == 3)
> +      return true;
> +    if ((*++*s != 'x') && use_xreg)
> +      return false;
> +    /* Reg4 could be any other sN register, where N > 1.  */
> +    if (!reg_lookup (s, RCLASS_GPR, &regno)
> +       || !(*rlist = regno_to_rlist (regno))
> +       || regno <= X_S2)
> +      return false;
> +    return true;
> +  }
> +
> +  free(reglist);

space before (

> +  return false;
> +}
> +
>  #define USE_BITS(mask,shift) (used_bits |= ((insn_t)(mask) << (shift)))
>  #define USE_IMM(n, s) \
>    (used_bits |= ((insn_t)((1ull<<n)-1) << (s)))
> diff --git a/include/opcode/riscv.h b/include/opcode/riscv.h
> index 710a9b73189..7b1ed47aa5d 100644
> --- a/include/opcode/riscv.h
> +++ b/include/opcode/riscv.h
> @@ -24,6 +24,7 @@
>  #include "riscv-opc.h"
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdio.h>

Why include stdio.h?