Re: [PATCH] riscv: thead: Add support for the XTheadInt ISA extension

2023-11-10 Thread Christoph Müllner
On Tue, Nov 7, 2023 at 4:04 AM Jin Ma  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 

[PATCH] riscv: thead: Add support for the XTheadInt ISA extension

2023-11-06 Thread Jin Ma
The XTheadInt ISA extension provides acceleration interruption
instructions as defined in T-Head-specific:

* th.ipush
* th.ipop

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);
+extern unsigned int th_int_get_save_adjustment();
+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.*/   \
+   && (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);
+  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. */
+  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);
+  if (th_int_mask && TH_INT_INTERRUPT (cfun))
+{
+  frame->mask &= ~th_int_mask;
+
+  /*