LGTM :)
On Fri, Jul 25, 2025 at 5:30 PM Christoph Müllner <
christoph.muell...@vrull.eu> wrote:
> There was once a RISC-V extension draft ("N"), which introduced
> user-level interrupts. However, it was never ratified and the
> specification draft has been removed from the RISC-V ISA manual
> in commit `b6cade07034` with the comment "it'll likely need to
> be redesigned".
>
> Support for a N extension never made it to GCC, but we support
> fuction attributes for user-level interrupt handlers that use
> the URET instruction.
>
> The "user" interrupt attribute was documented in the RISC-V C API,
> but has been removed in PR #106 in May 2025 (driven by LLVM devs/
> maintainers and ack'ed by at least one GCC maintainer).
>
> Let's drop URET support from GCC as well.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (enum riscv_privilege_levels): Remove
> USER_MODE.
> (riscv_handle_type_attribute): Remove "user" interrupts.
> (riscv_expand_epilogue): Likewise.
> (riscv_get_interrupt_type): Likewise.
> * config/riscv/riscv.md (riscv_uret): Remove URET pattern.
> * doc/extend.texi: Remove documentation of user interrupts.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/interrupt-conflict-mode.c: Remove "user"
> interrupts.
> * gcc.target/riscv/xtheadint-push-pop.c: Likewise.
> * gcc.target/riscv/interrupt-umode.c: Removed.
>
> Reported-by: Sam Elliott <quic_aelli...@quicinc.com>
> Signed-off-by: Christoph Müllner <christoph.muell...@vrull.eu>
> ---
> gcc/config/riscv/riscv.cc | 19 ++++++++-----------
> gcc/config/riscv/riscv.md | 8 --------
> gcc/doc/extend.texi | 4 ++--
> .../riscv/interrupt-conflict-mode.c | 2 +-
> .../gcc.target/riscv/interrupt-umode.c | 8 --------
> .../gcc.target/riscv/xtheadint-push-pop.c | 6 ------
> 6 files changed, 11 insertions(+), 36 deletions(-)
> delete mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-umode.c
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index b80d2a0efab4..b4f2d138efa9 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -170,7 +170,7 @@ struct GTY(()) riscv_frame_info {
> };
>
> enum riscv_privilege_levels {
> - UNKNOWN_MODE, USER_MODE, SUPERVISOR_MODE, MACHINE_MODE, RNMI_MODE
> + UNKNOWN_MODE, SUPERVISOR_MODE, MACHINE_MODE, RNMI_MODE
> };
>
> struct GTY(()) mode_switching_info {
> @@ -6929,11 +6929,12 @@ riscv_handle_type_attribute (tree *node
> ATTRIBUTE_UNUSED, tree name, tree args,
> error ("attribute 'rnmi' requires the Smrnmi ISA extension");
> *no_add_attrs = true;
> }
> - else if (strcmp (string, "user") && strcmp (string, "supervisor")
> - && strcmp (string, "machine") && strcmp (string, "rnmi"))
> + else if (strcmp (string, "supervisor")
> + && strcmp (string, "machine")
> + && strcmp (string, "rnmi"))
> {
> warning (OPT_Wattributes,
> - "argument to %qE attribute is not %<\"user\"%>,
> %<\"supervisor\"%>, "
> + "argument to %qE attribute is not
> %<\"supervisor\"%>, "
> "%<\"machine\"%>, or %<\"rnmi\"%>", name);
> *no_add_attrs = true;
> }
> @@ -9715,14 +9716,12 @@ riscv_expand_epilogue (int style)
>
> 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 ());
> else if (mode == RNMI_MODE)
> emit_jump_insn (gen_riscv_mnret ());
> - else
> - emit_jump_insn (gen_riscv_uret ());
> + else /* Must be MACHINE_MODE. */
> + emit_jump_insn (gen_riscv_mret ());
> }
> else if (style != SIBCALL_RETURN)
> {
> @@ -12064,9 +12063,7 @@ riscv_get_interrupt_type (tree decl)
> {
> const char *string = TREE_STRING_POINTER (TREE_VALUE (attr_args));
>
> - if (!strcmp (string, "user"))
> - return USER_MODE;
> - else if (!strcmp (string, "supervisor"))
> + if (!strcmp (string, "supervisor"))
> return SUPERVISOR_MODE;
> else if (!strcmp (string, "rnmi"))
> return RNMI_MODE;
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 0f29baa47e20..578dd43441e2 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -120,7 +120,6 @@ (define_c_enum "unspecv" [
> ;; Interrupt handler instructions.
> UNSPECV_MRET
> UNSPECV_SRET
> - UNSPECV_URET
> UNSPECV_MNRET
>
> ;; Blockage and synchronization.
> @@ -4167,13 +4166,6 @@ (define_insn "riscv_sret"
> "sret"
> [(set_attr "type" "ret")])
>
> -(define_insn "riscv_uret"
> - [(return)
> - (unspec_volatile [(const_int 0)] UNSPECV_URET)]
> - ""
> - "uret"
> - [(set_attr "type" "ret")])
> -
> (define_insn "riscv_mnret"
> [(return)
> (unspec_volatile [(const_int 0)] UNSPECV_MNRET)]
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 125eaf46331b..224d6197d639 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -5654,10 +5654,10 @@ You can specify the kind of interrupt to be
> handled by adding an optional
> parameter to the interrupt attribute like this:
>
> @smallexample
> -void f (void) __attribute__ ((interrupt ("user")));
> +void f (void) __attribute__ ((interrupt ("supervisor")));
> @end smallexample
>
> -Permissible values for this parameter are @code{user}, @code{supervisor},
> +Permissible values for this parameter are @code{supervisor},
> @code{machine}, and @code{rnmi}. If there is no parameter, then it
> defaults to @code{machine}.
>
> diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-conflict-mode.c
> b/gcc/testsuite/gcc.target/riscv/interrupt-conflict-mode.c
> index 81ebf5fba67d..15cc3eeb7304 100644
> --- a/gcc/testsuite/gcc.target/riscv/interrupt-conflict-mode.c
> +++ b/gcc/testsuite/gcc.target/riscv/interrupt-conflict-mode.c
> @@ -1,7 +1,7 @@
> /* Verify proper errors are generated for conflicted interrupt type. */
> /* { dg-do compile } */
> /* { dg-options "" } */
> -void __attribute__ ((interrupt ("user")))
> +void __attribute__ ((interrupt ("supervisor")))
> foo(void);
>
> void __attribute__ ((interrupt ("machine")))
> diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-umode.c
> b/gcc/testsuite/gcc.target/riscv/interrupt-umode.c
> deleted file mode 100644
> index 042abf02ad29..000000000000
> --- a/gcc/testsuite/gcc.target/riscv/interrupt-umode.c
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -/* Verify the return instruction is mret. */
> -/* { dg-do compile } */
> -/* { dg-options "" } */
> -void __attribute__ ((interrupt ("user")))
> -foo (void)
> -{
> -}
> -/* { dg-final { scan-assembler {\muret} } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c
> b/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c
> index dc5609c8f76e..167fa15843ce 100644
> --- a/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadint-push-pop.c
> @@ -20,12 +20,6 @@ void func_machine (void)
> /* { dg-final { scan-assembler-times {\mth\.ipop\M} 2 { target { rv32 } }
> } } */
>
>
> -__attribute__ ((interrupt ("user")))
> -void func_usr (void)
> -{
> - f ();
> -}
> -
> __attribute__ ((interrupt ("supervisor")))
> void func_supervisor (void)
> {
> --
> 2.50.1
>
>
