On 2023-05-31 18:13 Kito Cheng <kito.ch...@gmail.com> wrote: > >> >[1] >> >https://patchwork.sourceware.org/project/gcc/patch/20230406062118.47431-5-jia...@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" >)
Perfect. Working on it. > > > >> >> @@ -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. sure, I will make a separate patch. Thanks & BR, Fei
