On Tue, Nov 7, 2023 at 4:04 AM Jin Ma <ji...@linux.alibaba.com> wrote: > > The XTheadInt ISA extension provides acceleration interruption > instructions as defined in T-Head-specific: > > * th.ipush > * th.ipop
Overall, it looks ok to me. There are just a few small issues to clean up (see below). > > gcc/ChangeLog: > > * config/riscv/riscv-protos.h (th_int_get_mask): New prototype. > (th_int_get_save_adjustment): Likewise. > (th_int_adjust_cfi_prologue): Likewise. > * config/riscv/riscv.cc (TH_INT_INTERRUPT): New macro. > (riscv_expand_prologue): Add the processing of XTheadInt. > (riscv_expand_epilogue): Likewise. > * config/riscv/riscv.md: New unspec. > * config/riscv/thead.cc (BITSET_P): New macro. > * config/riscv/thead.md (th_int_push): New pattern. > (th_int_pop): New pattern. > > gcc/testsuite/ChangeLog: > > * gcc.target/riscv/xtheadint-push-pop.c: New test. > --- > gcc/config/riscv/riscv-protos.h | 3 + > gcc/config/riscv/riscv.cc | 58 +++++++++++++- > gcc/config/riscv/riscv.md | 4 + > gcc/config/riscv/thead.cc | 78 +++++++++++++++++++ > gcc/config/riscv/thead.md | 67 ++++++++++++++++ > .../gcc.target/riscv/xtheadint-push-pop.c | 36 +++++++++ > 6 files changed, 245 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h > index 85d4f6ed9ea..05d1fc2b3a0 100644 > --- a/gcc/config/riscv/riscv-protos.h > +++ b/gcc/config/riscv/riscv-protos.h > @@ -627,6 +627,9 @@ extern void th_mempair_prepare_save_restore_operands > (rtx[4], bool, > int, HOST_WIDE_INT, > int, HOST_WIDE_INT); > extern void th_mempair_save_restore_regs (rtx[4], bool, machine_mode); > +extern unsigned int th_int_get_mask(unsigned int); Space between function name and parenthesis. > +extern unsigned int th_int_get_save_adjustment(); Space between function name and parenthesis. An empty parameter list should be written as "(void)". > +extern rtx th_int_adjust_cfi_prologue (unsigned int); > #ifdef RTX_CODE > extern const char* > th_mempair_output_move (rtx[4], bool, machine_mode, RTX_CODE); > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc > index 08ff05dcc3f..c623101b05e 100644 > --- a/gcc/config/riscv/riscv.cc > +++ b/gcc/config/riscv/riscv.cc > @@ -101,6 +101,16 @@ along with GCC; see the file COPYING3. If not see > /* True the mode switching has static frm, or false. */ > #define STATIC_FRM_P(c) ((c)->machine->mode_sw_info.static_frm_p) > > +/* True if we can use the instructions in the XTheadInt extension > + to handle interrupts, or false. */ > +#define TH_INT_INTERRUPT(c) \ > + (TARGET_XTHEADINT \ > + /* The XTheadInt extension only supports rv32. */ \ > + && !TARGET_64BIT \ > + && (c)->machine->interrupt_handler_p \ > + /* This instruction can be executed in M-mode only.*/ \ Dot, space, space, end of comment. Maybe better: /* The XTheadInt instructions can only be executed in M-mode. */ > + && (c)->machine->interrupt_mode == MACHINE_MODE) > + > /* Information about a function's frame layout. */ > struct GTY(()) riscv_frame_info { > /* The size of the frame in bytes. */ > @@ -6703,6 +6713,7 @@ riscv_expand_prologue (void) > unsigned fmask = frame->fmask; > int spimm, multi_push_additional, stack_adj; > rtx insn, dwarf = NULL_RTX; > + unsigned th_int_mask = 0; > > if (flag_stack_usage_info) > current_function_static_stack_size = constant_lower_bound > (remaining_size); > @@ -6771,6 +6782,28 @@ riscv_expand_prologue (void) > REG_NOTES (insn) = dwarf; > } > > + th_int_mask = th_int_get_mask(frame->mask); There should be exactly one space between function name and parenthesis. > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask &= ~th_int_mask; > + > + /* RISCV_PROLOGUE_TEMP may be used to handle some CSR for > + interrupts, such as fcsr. */ Dot, space, space, end of comment. > + if ((TARGET_HARD_FLOAT && frame->fmask) > + || (TARGET_ZFINX && frame->mask)) > + frame->mask |= (1 << RISCV_PROLOGUE_TEMP_REGNUM); > + > + unsigned save_adjustment = th_int_get_save_adjustment (); > + frame->gp_sp_offset -= save_adjustment; > + remaining_size -= save_adjustment; > + > + insn = emit_insn (gen_th_int_push ()); > + > + rtx dwarf = th_int_adjust_cfi_prologue (th_int_mask); > + RTX_FRAME_RELATED_P (insn) = 1; > + REG_NOTES (insn) = dwarf; > + } > + > /* Save the GP, FP registers. */ > if ((frame->mask | frame->fmask) != 0) > { > @@ -6999,6 +7032,7 @@ riscv_expand_epilogue (int style) > = use_multi_pop ? frame->multi_push_adj_base + frame->multi_push_adj_addi > : 0; > rtx ra = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM); > + unsigned th_int_mask = 0; > rtx insn; > > /* We need to add memory barrier to prevent read from deallocated stack. > */ > @@ -7161,12 +7195,32 @@ riscv_expand_epilogue (int style) > else if (use_restore_libcall) > frame->mask = 0; /* Temporarily fib that we need not restore GPRs. */ > > + th_int_mask = th_int_get_mask(frame->mask); There should be exactly one space between function name and parenthesis. > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask &= ~th_int_mask; > + > + /* RISCV_PROLOGUE_TEMP may be used to handle some CSR for > + interrupts, such as fcsr. */ Dot, space, space, end of comment. > + if ((TARGET_HARD_FLOAT && frame->fmask) > + || (TARGET_ZFINX && frame->mask)) > + frame->mask |= (1 << RISCV_PROLOGUE_TEMP_REGNUM); > + } > + > /* Restore the registers. */ > riscv_for_each_saved_v_reg (step2, riscv_restore_reg, false); > riscv_for_each_saved_reg (frame->total_size - step2 - libcall_size > - multipop_size, > riscv_restore_reg, true, style == > EXCEPTION_RETURN); > > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + { > + frame->mask = mask; /* Undo the above fib. */ > + unsigned save_adjustment = th_int_get_save_adjustment ();; Double semi-colon. > + gcc_assert (step2.to_constant () >= save_adjustment); > + step2 -= save_adjustment; > + } > + > if (use_restore_libcall) > frame->mask = mask; /* Undo the above fib. */ > > @@ -7229,7 +7283,9 @@ riscv_expand_epilogue (int style) > > gcc_assert (mode != UNKNOWN_MODE); > > - if (mode == MACHINE_MODE) > + if (th_int_mask && TH_INT_INTERRUPT (cfun)) > + emit_jump_insn (gen_th_int_pop ()); > + else if (mode == MACHINE_MODE) > emit_jump_insn (gen_riscv_mret ()); > else if (mode == SUPERVISOR_MODE) > emit_jump_insn (gen_riscv_sret ()); > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md > index ae2217d0907..d4147d08838 100644 > --- a/gcc/config/riscv/riscv.md > +++ b/gcc/config/riscv/riscv.md > @@ -126,6 +126,10 @@ (define_c_enum "unspecv" [ > ;; XTheadFmv unspec > UNSPEC_XTHEADFMV > UNSPEC_XTHEADFMV_HW > + > + ;; XTheadInt unspec > + UNSPECV_XTHEADINT_PUSH > + UNSPECV_XTHEADINT_POP > ]) > > (define_constants > diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc > index a485fb1fba6..5b437c9a7a2 100644 > --- a/gcc/config/riscv/thead.cc > +++ b/gcc/config/riscv/thead.cc > @@ -36,6 +36,9 @@ > #include "regs.h" > #include "riscv-protos.h" > > +/* True if bit BIT is set in VALUE. */ > +#define BITSET_P(VALUE, BIT) (((VALUE) & (1ULL << (BIT))) != 0) > + This is copied from riscv.cc. Let's better move it to riscv.h instead of duplicating it. > /* If MEM is in the form of "base+offset", extract the two parts > of address and set to BASE and OFFSET, otherwise return false > after clearing BASE and OFFSET. */ > @@ -945,3 +948,78 @@ th_print_operand_address (FILE *file, machine_mode mode, > rtx x) > > gcc_unreachable (); > } > + > +/* Number array of registers X1, X5-X7, X10-X17, X28-X31, to be > + operated on by instruction th.ipush/th.ipop in XTheadInt. */ > + > +int th_int_regs[] ={ > + RETURN_ADDR_REGNUM, > + T0_REGNUM, T1_REGNUM, T2_REGNUM, > + A0_REGNUM, A1_REGNUM, A2_REGNUM, A3_REGNUM, > + A4_REGNUM, A5_REGNUM, A6_REGNUM, A7_REGNUM, > + T3_REGNUM, T4_REGNUM, T5_REGNUM, T6_REGNUM, > +}; > + > +/* If MASK contains registers X1, X5-X7, X10-X17, X28-X31, then > + return the mask composed of these registers, otherwise return > + zero. */ > + > +unsigned int > +th_int_get_mask(unsigned int mask) There should be exactly one space between function name and parenthesis. > +{ > + unsigned int xtheadint_mask = 0; > + > + if (!TARGET_XTHEADINT || TARGET_64BIT) > + return 0; > + > + for (unsigned int i = 0; i < ARRAY_SIZE (th_int_regs); i++) > + { > + if (!BITSET_P (mask, th_int_regs[i])) > + return 0; > + > + xtheadint_mask |= (1 << th_int_regs[i]); > + } > + > + return xtheadint_mask; /* Usually 0xf003fce2. */ > +} > + > +/* Returns the occupied frame needed to save registers X1, X5-X7, > + X10-X17, X28-X31.*/ Dot, space, space, end of comment. > + > +unsigned int > +th_int_get_save_adjustment () "()" -> "(void)" > +{ > + gcc_assert (TARGET_XTHEADINT && !TARGET_64BIT); > + return ARRAY_SIZE (th_int_regs) * UNITS_PER_WORD; > +} > + > +rtx > +th_int_adjust_cfi_prologue (unsigned int mask) > +{ > + gcc_assert (TARGET_XTHEADINT && !TARGET_64BIT); > + > + rtx dwarf = NULL_RTX; > + rtx adjust_sp_rtx, reg, mem, insn; > + int saved_size = ARRAY_SIZE (th_int_regs) * UNITS_PER_WORD; > + int offset = saved_size; > + > + for (int regno = GP_REG_FIRST; regno <= GP_REG_LAST; regno++) > + if (BITSET_P (mask, regno - GP_REG_FIRST)) > + { > + offset -= UNITS_PER_WORD; > + reg = gen_rtx_REG (SImode, regno); > + mem = gen_frame_mem (SImode, plus_constant (Pmode, > + stack_pointer_rtx, > + offset)); > + > + insn = gen_rtx_SET (mem, reg); > + dwarf = alloc_reg_note (REG_CFA_OFFSET, insn, dwarf); > + } > + > + /* Debug info for adjust sp. */ > + adjust_sp_rtx = gen_rtx_SET (stack_pointer_rtx, > + gen_rtx_PLUS (GET_MODE(stack_pointer_rtx), stack_pointer_rtx, GEN_INT > (-saved_size))); There should be exactly one space between function name and parenthesis. > + dwarf = alloc_reg_note (REG_CFA_ADJUST_CFA, adjust_sp_rtx, dwarf); > + > + return dwarf; > +} > diff --git a/gcc/config/riscv/thead.md b/gcc/config/riscv/thead.md > index 2babfafb23c..4d6e16c0edc 100644 > --- a/gcc/config/riscv/thead.md > +++ b/gcc/config/riscv/thead.md > @@ -210,6 +210,73 @@ (define_insn "th_fmv_x_hw" > (set_attr "type" "fmove") > (set_attr "mode" "DF")]) > > +;; XTheadInt > + > +(define_constants > + [(T0_REGNUM 5) > + (T1_REGNUM 6) > + (T2_REGNUM 7) > + (A0_REGNUM 10) > + (A1_REGNUM 11) > + (A2_REGNUM 12) > + (A3_REGNUM 13) > + (A4_REGNUM 14) > + (A5_REGNUM 15) > + (A6_REGNUM 16) > + (A7_REGNUM 17) > + (T3_REGNUM 28) > + (T4_REGNUM 29) > + (T5_REGNUM 30) > + (T6_REGNUM 31) > +]) > + > +(define_insn "th_int_push" > + [(unspec_volatile [(const_int 0)] UNSPECV_XTHEADINT_PUSH) > + (use (reg:SI RETURN_ADDR_REGNUM)) > + (use (reg:SI T0_REGNUM)) > + (use (reg:SI T1_REGNUM)) > + (use (reg:SI T2_REGNUM)) > + (use (reg:SI A0_REGNUM)) > + (use (reg:SI A1_REGNUM)) > + (use (reg:SI A2_REGNUM)) > + (use (reg:SI A3_REGNUM)) > + (use (reg:SI A4_REGNUM)) > + (use (reg:SI A5_REGNUM)) > + (use (reg:SI A6_REGNUM)) > + (use (reg:SI A7_REGNUM)) > + (use (reg:SI T3_REGNUM)) > + (use (reg:SI T4_REGNUM)) > + (use (reg:SI T5_REGNUM)) > + (use (reg:SI T6_REGNUM))] > + "TARGET_XTHEADINT && !TARGET_64BIT" > + "th.ipush" > + [(set_attr "type" "store") > + (set_attr "mode" "SI")]) > + > +(define_insn "th_int_pop" > + [(unspec_volatile [(const_int 0)] UNSPECV_XTHEADINT_POP) > + (clobber (reg:SI RETURN_ADDR_REGNUM)) > + (clobber (reg:SI T0_REGNUM)) > + (clobber (reg:SI T1_REGNUM)) > + (clobber (reg:SI T2_REGNUM)) > + (clobber (reg:SI A0_REGNUM)) > + (clobber (reg:SI A1_REGNUM)) > + (clobber (reg:SI A2_REGNUM)) > + (clobber (reg:SI A3_REGNUM)) > + (clobber (reg:SI A4_REGNUM)) > + (clobber (reg:SI A5_REGNUM)) > + (clobber (reg:SI A6_REGNUM)) > + (clobber (reg:SI A7_REGNUM)) > + (clobber (reg:SI T3_REGNUM)) > + (clobber (reg:SI T4_REGNUM)) > + (clobber (reg:SI T5_REGNUM)) > + (clobber (reg:SI T6_REGNUM)) > + (return)] > + "TARGET_XTHEADINT && !TARGET_64BIT" > + "th.ipop" > + [(set_attr "type" "ret") > + (set_attr "mode" "SI")]) > + > ;; XTheadMac > > (define_insn "*th_mula<mode>" > diff --git a/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > b/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > new file mode 100644 > index 00000000000..3383431aa30 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c > @@ -0,0 +1,36 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=rv32gc_xtheadint -mabi=ilp32d" { target { rv32 } } } > */ > +/* { dg-options "-march=rv64gc_xtheadint -mabi=lp64d" { target { rv64 } } } > */ > + > +extern void f(void); > + > +__attribute__((interrupt)) > +void func_default(void) There should be exactly one space between function name and parenthesis. > +{ > + f (); > +} > + > +__attribute__ ((interrupt ("machine"))) > +void func_machine(void) There should be exactly one space between function name and parenthesis. > +{ > + f (); > +} > + > +/* { dg-final { scan-assembler-times {\tth\.ipush\n} 2 { target { rv32 } } } > } */ > +/* { dg-final { scan-assembler-times {\tth\.ipop\n} 2 { target { rv32 } } } > } */ > + > + > +__attribute__ ((interrupt ("user"))) > +void func_usr(void) > +{ > + f (); > +} > + > +__attribute__ ((interrupt ("supervisor"))) > +void func_supervisor(void) > +{ > + f (); > +} > + > +/* { dg-final { scan-assembler-not {\tth\.ipush\n} { target { rv64 } } } } */ > +/* { dg-final { scan-assembler-not {\tth\.ipop\n} { target { rv64 } } } } */ > \ No newline at end of file > > base-commit: 2cca6ae615f3fb083d3a1e5e9dffcefd54fed990 > -- > 2.17.1 >