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

[jiawei@iscas.ac.cn]

Thanks for your suggestions, fixed in new patches.

> -----原始邮件-----
> 发件人: "Kito Cheng" <kito.cheng@sifive.com>
> 发送时间: 2024-01-30 21:27:54 (星期二)
> 收件人: Jiawei <jiawei@iscas.ac.cn>
> 抄送: binutils@sourceware.org, nelson@rivosinc.com, palmer@dabbelt.com, jbeulich@suse.com, research_trasio@irq.a4lg.com, christoph.muellner@vrull.eu, jeremy.bennett@embecosm.com, nandni.jamnadas@embecosm.com, mary.bennett@embecosm.com, charlie.keaney@embecosm.com, simon.cook@embecosm.com, sinan.lin@linux.alibaba.com, gaofei@eswincomputing.com, fujin.zhao@foxmail.com, wuwei2016@iscas.ac.cn, shihua@iscas.ac.cn, shiyulong@iscas.ac.cn, chenyixuan@iscas.ac.cn
> 主题: Re: [PATCH v5 1/2] RISC-V: Support Zcmp push/pop instructions.
> 
> > +static bool
> > +reglist_lookup (char **s, unsigned *reg_list)
> > +{
> > +  unsigned regno = 0;
> > +  unsigned regnum = 0;
> > +  char *reglist = strdup (*s);
> 
> ^^^^ this is leaked in many place
> 
> > +  char *regname[3];
> > +
> > +  if (reglist == NULL)
> > +    return false;
> > +
> > +  reglist = strtok (reglist, "}");
> 
> ^^^^ you need to back up the original reglist pointer for free it.

I'm not sure here, when I try to use two, they still keep in one pointer.

> 
> > +  for(reglist = strtok (reglist, ",");reglist;reglist = strtok(NULL, ",")){
> > +    regname[regnum] = reglist;
> > +    regnum++;
> > +  }
> > +
> > +  /* Use to check if the register format is xreg.  */
> > +  bool use_xreg = **s == 'x';
> > +
> > +  /* The first register in the register list should be ra.  */
> > +  if (!reg_lookup (s, RCLASS_GPR, ®no)
> > +     || !(*reg_list = regno_to_reg_list (regno)) /* update reg_list */
> > +     || regno != X_RA)
> > +    return false;
> 
> reglist leak here.
> 
> > +
> > +  if (regnum == 1)
> > +    return true;
> 
> reglist leak here.
> 
> > +
> > +  /* Do not use numeric and abi names at the same time.  */
> > +  if ((*++*s != 'x') && use_xreg)
> > +    return false;
> 
> reglist leak here.
> 
> > +  /* Reg1 should be s0 or its numeric names x8.  */
> > +  if (!reg_lookup (s, RCLASS_GPR, ®no)
> > +     || !(*reg_list = regno_to_reg_list (regno))
> > +     || regno != X_S0)
> > +    return false;
> 
> reglist leak here.
> 
> > +
> > +  if (strlen (regname[1]) == 2)
> > +    return true;
> > +
> > +  if ((*++*s != 'x') && use_xreg)
> > +    return false;
> 
> 
> reglist leak here.
> 
> > +  /* 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, ®no)
> > +     || !(*reg_list = regno_to_reg_list (regno))
> > +     || regno <= X_S0
> > +     || (use_xreg && regno != X_S1))
> > +    return false;
> 
> reglist leak here.
> 
> > +
> > +  if (regnum == 2)
> > +    return true;
> 
> reglist leak here.
> 
> > +
> > +  if (regnum == 3 && use_xreg) {
> > +    if ((*++*s != 'x') && use_xreg)
> > +      return false;
> 
> reglist leak here.
> 
> > +    /* Reg3 should be s2.  */
> > +    if (!reg_lookup (s, RCLASS_GPR, ®no)
> > +       || !(*reg_list = regno_to_reg_list (regno))
> > +       || regno != X_S2)
> > +      return false;
> 
> reglist leak here.
> 
> > +    if(strlen(regname[2]) == 3)
> > +      return true;
> 
> reglist leak here.
> 
> > +    if ((*++*s != 'x') && use_xreg)
> > +      return false;
> 
> reglist leak here.
> 
> > +    /* Reg4 could be any other sN register, where N > 1.  */
> > +    if (!reg_lookup (s, RCLASS_GPR, ®no)
> > +       || !(*reg_list = regno_to_reg_list (regno))
> > +       || regno <= X_S2)
> > +      return false;
> > +    return true;
> 
> 
> reglist leak here.
> 
> > +  }
> > +
> 
> I guess you may goto to make it easier.
> 
> e.g.
> 
> ret = true;
> goto done;
> ...
> ...
> free (reglist);
> return ret;
> 
> > +  free (reglist);
> > +  return false;
> > +}
> > +
> >  #define USE_BITS(mask,shift) (used_bits |= ((insn_t)(mask) << (shift)))
> >  #define USE_IMM(n, s) \
> >    (used_bits |= ((insn_t)((1ull<</jiawei@iscas.ac.cn></kito.cheng@sifive.com>