Re: Re: [PATCH 1/1] [V2] [RISC-V] support cm.push cm.pop cm.popret in zcmp

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

> >[1] https://patchwork.sourceware.org/project/gcc/patch/20230406062118.47431-5-jiawei@iscas.ac.cn/
> Thanks for your review.
>
> The md file looks verbose with bunch of *_offset_operand and stack_push_up_to_*_operand, but it significantly
> simplies implementation of recognizing zmcp push and pop insns and outputting assembly.  Also, the md file
> clearly shows and checks the slot that each register is placed(different to slot order w/o save-restore before
> zcmp is introduced). So I prefer my patch V2 to V1 or the link you attached. But ideas are welcome to make
> it better. Appreciated if you suggest more details for the improvement.

Got your point, and share an idea to simplify that:

struct code_for_push_pop_t {
   insn_code (*push)(machine_mode);
   insn_code (*pop)(machine_mode);
   insn_code (*pop_ret)(machine_mode);
};
const code_for_push_pop_t code_for_push_pop [/*ZCMP_MAX_GRP_SLOTS*/2] = {
    {code_for_gpr_multi_pop_up_to_ra, /*FIXME*/nullptr, /*FIXME*/nullptr},
    {code_for_gpr_multi_pop_up_to_s0, /*FIXME*/nullptr, /*FIXME*/nullptr}
};

static rtx
riscv_gen_multi_push_pop_insn (op_idx op, HOST_WIDE_INT adj_size,
unsigned int regs_num)
{
  rtx stack_adj = GEN_INT (adj_size);

  return GEN_FCN (code_for_push_pop[regs_num].push(Pmode)) (stack_adj);
}

(define_mode_attr slot0_offset [(SI "0") (DI "0")])
(define_mode_attr slot1_offset [(SI "4") (DI "8")])

(define_insn "@gpr_multi_pop_up_to_ra<mode>"
  [(set (reg:X SP_REGNUM)
        (plus:X (reg:X SP_REGNUM)
                 (match_operand 0 "stack_pop_up_to_ra_operand" "I")))
   (set (reg:X RETURN_ADDR_REGNUM)
        (mem:X (plus:X (reg:X SP_REGNUM)
                       (const_int <slot0_offset>))))]
  "TARGET_ZCMP"
  "cm.pop       {ra}, %0"
)

(define_insn "@gpr_multi_pop_up_to_s0<mode>"
  [(set (reg:X SP_REGNUM)
        (plus:X (reg:X SP_REGNUM)
                 (match_operand 0 "stack_pop_up_to_s0_operand" "I")))
   (set (reg:X S0_REGNUM)
        (mem:X (plus:X (reg:X SP_REGNUM)
                       (const_int <slot0_offset>))))
   (set (reg:X RETURN_ADDR_REGNUM)
        (mem:X (plus:X (reg:X SP_REGNUM)
                       (const_int <slot1_offset>))))]
  "TARGET_ZCMP"
  "cm.pop       {ra, s0}, %0"
)



> >> @@ -5620,7 +5977,7 @@ riscv_expand_epilogue (int style)
> >>                                            adjust));
> >>           rtx dwarf = NULL_RTX;
> >>           rtx cfa_adjust_rtx = gen_rtx_PLUS (Pmode, stack_pointer_rtx,
> >> -                                            GEN_INT (step2));
> >> +                                            GEN_INT (step2 + libcall_size + multipop_size));
> >
> >Why we need `+ libcall_size` here? or...why we don't need that before?
> It's a good catch:)
> I should have  added `+ libcall_size` in
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=60524be1e3929d83e15fceac6e2aa053c8a6fb20
>
> That's why I corrected the cfi issue in save-restore along with zcmp changes in this patch.

I would like to have a separate patch to fix this bug instead of
hidden in this patch.