RISC-V: Fix interrupt support for -g.

This fixes a problem found by someone trying to use the new RISC-V interrupt
attribute support.  With a slightly non-trivial example, and the -g option, we
get an abort in dwarf2cfi for an inconsistent CFI state.  This is my fault
for not making the new interrupt return patterns look enough like regular
return patterns, which is simple to fix.

Tested with cross riscv32-elf and riscv64-linux builds and tests.  There were
no regressions.  The new testcase fails without the patch and works with the
patch.

Committed.

Jim

	gcc/
	* config/riscv/riscv.c (riscv_expand_epilogue): Use emit_jump_insn
	instead of emit_insn for interrupt returns.
	* config/riscv/riscv.md (riscv_met): Add (return) to rtl.
	(riscv_sret, riscv_uret): Likewise.

	gcc/testsuite/
	* gcc.target/riscv/interrupt-debug.c: New.
---
 gcc/config/riscv/riscv.c                         |  6 +++---
 gcc/config/riscv/riscv.md                        |  9 ++++++---
 gcc/testsuite/gcc.target/riscv/interrupt-debug.c | 15 +++++++++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-debug.c

Hi Jim:

Does it possible just combine those pattern into simple_return
pattern, and then check the function type and output correct return
instruction in riscv_output_return?
On Tue, Jul 3, 2018 at 8:22 AM Jim Wilson <jimw@sifive.com> wrote:
>
> This fixes a problem found by someone trying to use the new RISC-V interrupt
> attribute support.  With a slightly non-trivial example, and the -g option, we
> get an abort in dwarf2cfi for an inconsistent CFI state.  This is my fault
> for not making the new interrupt return patterns look enough like regular
> return patterns, which is simple to fix.
>
> Tested with cross riscv32-elf and riscv64-linux builds and tests.  There were
> no regressions.  The new testcase fails without the patch and works with the
> patch.
>
> Committed.
>
> Jim
>
>         gcc/
>         * config/riscv/riscv.c (riscv_expand_epilogue): Use emit_jump_insn
>         instead of emit_insn for interrupt returns.
>         * config/riscv/riscv.md (riscv_met): Add (return) to rtl.
>         (riscv_sret, riscv_uret): Likewise.
>
>         gcc/testsuite/
>         * gcc.target/riscv/interrupt-debug.c: New.
> ---
>  gcc/config/riscv/riscv.c                         |  6 +++---
>  gcc/config/riscv/riscv.md                        |  9 ++++++---
>  gcc/testsuite/gcc.target/riscv/interrupt-debug.c | 15 +++++++++++++++
>  3 files changed, 24 insertions(+), 6 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/interrupt-debug.c
>
> diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
> index 2709ebdd797..d87836f53f8 100644
> --- a/gcc/config/riscv/riscv.c
> +++ b/gcc/config/riscv/riscv.c
> @@ -3985,11 +3985,11 @@ riscv_expand_epilogue (int style)
>        enum riscv_privilege_levels mode = cfun->machine->interrupt_mode;
>
>        if (mode == MACHINE_MODE)
> -       emit_insn (gen_riscv_mret ());
> +       emit_jump_insn (gen_riscv_mret ());
>        else if (mode == SUPERVISOR_MODE)
> -       emit_insn (gen_riscv_sret ());
> +       emit_jump_insn (gen_riscv_sret ());
>        else
> -       emit_insn (gen_riscv_uret ());
> +       emit_jump_insn (gen_riscv_uret ());
>      }
>    else if (style != SIBCALL_RETURN)
>      emit_jump_insn (gen_simple_return_internal (ra));
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 7b411f0538e..613af9d79e4 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -2328,17 +2328,20 @@
>    "fsflags\t%0")
>
>  (define_insn "riscv_mret"
> -  [(unspec_volatile [(const_int 0)] UNSPECV_MRET)]
> +  [(return)
> +   (unspec_volatile [(const_int 0)] UNSPECV_MRET)]
>    ""
>    "mret")
>
>  (define_insn "riscv_sret"
> -  [(unspec_volatile [(const_int 0)] UNSPECV_SRET)]
> +  [(return)
> +   (unspec_volatile [(const_int 0)] UNSPECV_SRET)]
>    ""
>    "sret")
>
>  (define_insn "riscv_uret"
> -  [(unspec_volatile [(const_int 0)] UNSPECV_URET)]
> +  [(return)
> +   (unspec_volatile [(const_int 0)] UNSPECV_URET)]
>    ""
>    "uret")
>
> diff --git a/gcc/testsuite/gcc.target/riscv/interrupt-debug.c b/gcc/testsuite/gcc.target/riscv/interrupt-debug.c
> new file mode 100644
> index 00000000000..a1b6dac8fbb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/interrupt-debug.c
> @@ -0,0 +1,15 @@
> +/* Verify that we can compile with debug info.  */
> +/* { dg-do compile } */
> +/* { dg-options "-Og -g" } */
> +extern int var1;
> +extern int var2;
> +extern void sub2 (void);
> +
> +void __attribute__ ((interrupt))
> +sub (void)
> +{
> +  if (var1)
> +    var2 = 0;
> +  else
> +    sub2 ();
> +}
> --
> 2.17.1
>

On Mon, Jul 2, 2018 at 8:04 PM, Kito Cheng <kito.cheng@gmail.com> wrote:
> Does it possible just combine those pattern into simple_return
> pattern, and then check the function type and output correct return
> instruction in riscv_output_return?

There might be problems with optimizations thinking this is a regular
return when it isn't.  But it might work if we have enough checks for
the interrupt attribute in the right places.

Having different patterns for different types of return instructions
makes the RTL dumps self contained.  You can tell what kind of
instruction it is without having to look at the source tree to see how
the function was declared.  There is some minor benefit to that.

Is there a problem with the current approach that needs fixing?

Jim

Hi Jim:

It's no problem with current approach, I just think it can simplify
the .md file.

Thanks :)
On Tue, Jul 3, 2018 at 11:22 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Mon, Jul 2, 2018 at 8:04 PM, Kito Cheng <kito.cheng@gmail.com> wrote:
> > Does it possible just combine those pattern into simple_return
> > pattern, and then check the function type and output correct return
> > instruction in riscv_output_return?
>
> There might be problems with optimizations thinking this is a regular
> return when it isn't.  But it might work if we have enough checks for
> the interrupt attribute in the right places.
>
> Having different patterns for different types of return instructions
> makes the RTL dumps self contained.  You can tell what kind of
> instruction it is without having to look at the source tree to see how
> the function was declared.  There is some minor benefit to that.
>
> Is there a problem with the current approach that needs fixing?
>
> Jim