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

[jiawei@iscas.ac.cn]

Thanks for your suggestions, fixed in new patches.

> -----原始邮件-----
&gt; 发件人: "Kito Cheng" <kito.cheng@sifive.com>
&gt; 发送时间: 2024-01-30 21:27:54 (星期二)
&gt; 收件人: Jiawei <jiawei@iscas.ac.cn>
&gt; 抄送: 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
&gt; 主题: Re: [PATCH v5 1/2] RISC-V: Support Zcmp push/pop instructions.
&gt; 
&gt; &gt; +static bool
&gt; &gt; +reglist_lookup (char **s, unsigned *reg_list)
&gt; &gt; +{
&gt; &gt; +  unsigned regno = 0;
&gt; &gt; +  unsigned regnum = 0;
&gt; &gt; +  char *reglist = strdup (*s);
&gt; 
&gt; ^^^^ this is leaked in many place
&gt; 
&gt; &gt; +  char *regname[3];
&gt; &gt; +
&gt; &gt; +  if (reglist == NULL)
&gt; &gt; +    return false;
&gt; &gt; +
&gt; &gt; +  reglist = strtok (reglist, "}");
&gt; 
&gt; ^^^^ 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.

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