[v2] RISC-V: Fixbug for that XTheadMemPair causes interrupt to fail.

The t0 register is used as a temporary register for interrupts, so it needs
special treatment. It is necessary to avoid using "th.ldd" in the interrupt
program to stop the subsequent operation of the t0 register, so they need to
exchange positions in the function "riscv_for_each_saved_reg".

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_for_each_saved_reg): Place the interrupt
	operation before the XTheadMemPair.
---
 gcc/config/riscv/riscv.cc                     | 56 +++++++++----------
 .../riscv/xtheadmempair-interrupt-fcsr.c      | 18 ++++++
 2 files changed, 46 insertions(+), 28 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c


base-commit: e7f4040d9d6ec40c48ada940168885d7dde03af9

I thought Christoph was already committed? Do you mind describing the
difference between v1 and v2?

On Fri, Nov 10, 2023 at 9:55 PM Jin Ma <jinma@linux.alibaba.com> wrote:

> The t0 register is used as a temporary register for interrupts, so it needs
> special treatment. It is necessary to avoid using "th.ldd" in the interrupt
> program to stop the subsequent operation of the t0 register, so they need
> to
> exchange positions in the function "riscv_for_each_saved_reg".
>
> gcc/ChangeLog:
>
>         * config/riscv/riscv.cc (riscv_for_each_saved_reg): Place the
> interrupt
>         operation before the XTheadMemPair.
> ---
>  gcc/config/riscv/riscv.cc                     | 56 +++++++++----------
>  .../riscv/xtheadmempair-interrupt-fcsr.c      | 18 ++++++
>  2 files changed, 46 insertions(+), 28 deletions(-)
>  create mode 100644
> gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index e25692b86fc..fa2d4d4b779 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -6346,6 +6346,34 @@ riscv_for_each_saved_reg (poly_int64 sp_offset,
> riscv_save_restore_fn fn,
>           && riscv_is_eh_return_data_register (regno))
>         continue;
>
> +      /* In an interrupt function, save and restore some necessary CSRs
> in the stack
> +        to avoid changes in CSRs.  */
> +      if (regno == RISCV_PROLOGUE_TEMP_REGNUM
> +         && cfun->machine->interrupt_handler_p
> +         && ((TARGET_HARD_FLOAT  && cfun->machine->frame.fmask)
> +             || (TARGET_ZFINX
> +                 && (cfun->machine->frame.mask & ~(1 <<
> RISCV_PROLOGUE_TEMP_REGNUM)))))
> +       {
> +         unsigned int fcsr_size = GET_MODE_SIZE (SImode);
> +         if (!epilogue)
> +           {
> +             riscv_save_restore_reg (word_mode, regno, offset, fn);
> +             offset -= fcsr_size;
> +             emit_insn (gen_riscv_frcsr (RISCV_PROLOGUE_TEMP (SImode)));
> +             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
> +                                     offset, riscv_save_reg);
> +           }
> +         else
> +           {
> +             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
> +                                     offset - fcsr_size,
> riscv_restore_reg);
> +             emit_insn (gen_riscv_fscsr (RISCV_PROLOGUE_TEMP (SImode)));
> +             riscv_save_restore_reg (word_mode, regno, offset, fn);
> +             offset -= fcsr_size;
> +           }
> +         continue;
> +       }
> +
>        if (TARGET_XTHEADMEMPAIR)
>         {
>           /* Get the next reg/offset pair.  */
> @@ -6376,34 +6404,6 @@ riscv_for_each_saved_reg (poly_int64 sp_offset,
> riscv_save_restore_fn fn,
>             }
>         }
>
> -      /* In an interrupt function, save and restore some necessary CSRs
> in the stack
> -        to avoid changes in CSRs.  */
> -      if (regno == RISCV_PROLOGUE_TEMP_REGNUM
> -         && cfun->machine->interrupt_handler_p
> -         && ((TARGET_HARD_FLOAT  && cfun->machine->frame.fmask)
> -             || (TARGET_ZFINX
> -                 && (cfun->machine->frame.mask & ~(1 <<
> RISCV_PROLOGUE_TEMP_REGNUM)))))
> -       {
> -         unsigned int fcsr_size = GET_MODE_SIZE (SImode);
> -         if (!epilogue)
> -           {
> -             riscv_save_restore_reg (word_mode, regno, offset, fn);
> -             offset -= fcsr_size;
> -             emit_insn (gen_riscv_frcsr (RISCV_PROLOGUE_TEMP (SImode)));
> -             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
> -                                     offset, riscv_save_reg);
> -           }
> -         else
> -           {
> -             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
> -                                     offset - fcsr_size,
> riscv_restore_reg);
> -             emit_insn (gen_riscv_fscsr (RISCV_PROLOGUE_TEMP (SImode)));
> -             riscv_save_restore_reg (word_mode, regno, offset, fn);
> -             offset -= fcsr_size;
> -           }
> -         continue;
> -       }
> -
>        riscv_save_restore_reg (word_mode, regno, offset, fn);
>      }
>
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
> b/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
> new file mode 100644
> index 00000000000..d06f05f5c7c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
> @@ -0,0 +1,18 @@
> +/* Verify that fcsr instructions emitted.  */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target hard_float } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
> +/* { dg-options "-march=rv64gc_xtheadmempair -mtune=thead-c906
> -funwind-tables" { target { rv64 } } } */
> +/* { dg-options "-march=rv32gc_xtheadmempair -mtune=thead-c906
> -funwind-tables" { target { rv32 } } } */
> +
> +
> +extern int foo (void);
> +
> +void __attribute__ ((interrupt))
> +sub (void)
> +{
> +  foo ();
> +}
> +
> +/* { dg-final { scan-assembler-times "frcsr\t" 1 } } */
> +/* { dg-final { scan-assembler-times "fscsr\t" 1 } } */
>
> base-commit: e7f4040d9d6ec40c48ada940168885d7dde03af9
> --
> 2.17.1
>
>

I'm very sorry, I misunderstood. There's no difference between them, please ignore it.





  	
 马进 
阿里巴巴及蚂蚁集团  
 电话：057128223456-89384085 
 邮箱：yaofang.mj@alibaba-inc.com 
 地址：浙江-杭州-西溪B区 B2-7-E6-090 
	     
	   		 阿里巴巴及蚂蚁集团  	 企业主页  		      
 信息安全声明：本邮件包含信息归发件人所在组织所有，发件人所在组织对该邮件拥有所有权利。
请接收者注意保密，未经发件人书面许可，不得向任何第三方组织和个人透露本邮件所含信息的全部或部分。以上声明仅适用于工作邮件。
Information Security Notice: The information contained in this mail is solely property of the sender's organization. 
 This mail communication is confidential. Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.  ------------------------------------------------------------------
发件人：Kito Cheng<kito.cheng@gmail.com>
日　期：2023年11月10日 22:04:26
收件人：Jin Ma<jinma@linux.alibaba.com>
抄　送：<gcc-patches@gcc.gnu.org>; <christoph.muellner@vrull.eu>; <jinma.contrib@gmail.com>
主　题：Re: [PATCH v2] RISC-V: Fixbug for that XTheadMemPair causes interrupt to fail.

I thought Christoph was already committed? Do you mind describing the difference between v1 and v2?
On Fri, Nov 10, 2023 at 9:55 PM Jin Ma <jinma@linux.alibaba.com> wrote:
The t0 register is used as a temporary register for interrupts, so it needs
 special treatment. It is necessary to avoid using "th.ldd" in the interrupt
 program to stop the subsequent operation of the t0 register, so they need to
 exchange positions in the function "riscv_for_each_saved_reg".

 gcc/ChangeLog:

         * config/riscv/riscv.cc (riscv_for_each_saved_reg): Place the interrupt
         operation before the XTheadMemPair.
 ---
  gcc/config/riscv/riscv.cc                     | 56 +++++++++----------
  .../riscv/xtheadmempair-interrupt-fcsr.c      | 18 ++++++
  2 files changed, 46 insertions(+), 28 deletions(-)
  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c

 diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
 index e25692b86fc..fa2d4d4b779 100644
 --- a/gcc/config/riscv/riscv.cc
 +++ b/gcc/config/riscv/riscv.cc
 @@ -6346,6 +6346,34 @@ riscv_for_each_saved_reg (poly_int64 sp_offset, riscv_save_restore_fn fn,
           && riscv_is_eh_return_data_register (regno))
         continue;

 +      /* In an interrupt function, save and restore some necessary CSRs in the stack
 +        to avoid changes in CSRs.  */
 +      if (regno == RISCV_PROLOGUE_TEMP_REGNUM
 +         && cfun->machine->interrupt_handler_p
 +         && ((TARGET_HARD_FLOAT  && cfun->machine->frame.fmask)
 +             || (TARGET_ZFINX
 +                 && (cfun->machine->frame.mask & ~(1 << RISCV_PROLOGUE_TEMP_REGNUM)))))
 +       {
 +         unsigned int fcsr_size = GET_MODE_SIZE (SImode);
 +         if (!epilogue)
 +           {
 +             riscv_save_restore_reg (word_mode, regno, offset, fn);
 +             offset -= fcsr_size;
 +             emit_insn (gen_riscv_frcsr (RISCV_PROLOGUE_TEMP (SImode)));
 +             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
 +                                     offset, riscv_save_reg);
 +           }
 +         else
 +           {
 +             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
 +                                     offset - fcsr_size, riscv_restore_reg);
 +             emit_insn (gen_riscv_fscsr (RISCV_PROLOGUE_TEMP (SImode)));
 +             riscv_save_restore_reg (word_mode, regno, offset, fn);
 +             offset -= fcsr_size;
 +           }
 +         continue;
 +       }
 +
        if (TARGET_XTHEADMEMPAIR)
         {
           /* Get the next reg/offset pair.  */
 @@ -6376,34 +6404,6 @@ riscv_for_each_saved_reg (poly_int64 sp_offset, riscv_save_restore_fn fn,
             }
         }

 -      /* In an interrupt function, save and restore some necessary CSRs in the stack
 -        to avoid changes in CSRs.  */
 -      if (regno == RISCV_PROLOGUE_TEMP_REGNUM
 -         && cfun->machine->interrupt_handler_p
 -         && ((TARGET_HARD_FLOAT  && cfun->machine->frame.fmask)
 -             || (TARGET_ZFINX
 -                 && (cfun->machine->frame.mask & ~(1 << RISCV_PROLOGUE_TEMP_REGNUM)))))
 -       {
 -         unsigned int fcsr_size = GET_MODE_SIZE (SImode);
 -         if (!epilogue)
 -           {
 -             riscv_save_restore_reg (word_mode, regno, offset, fn);
 -             offset -= fcsr_size;
 -             emit_insn (gen_riscv_frcsr (RISCV_PROLOGUE_TEMP (SImode)));
 -             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
 -                                     offset, riscv_save_reg);
 -           }
 -         else
 -           {
 -             riscv_save_restore_reg (SImode, RISCV_PROLOGUE_TEMP_REGNUM,
 -                                     offset - fcsr_size, riscv_restore_reg);
 -             emit_insn (gen_riscv_fscsr (RISCV_PROLOGUE_TEMP (SImode)));
 -             riscv_save_restore_reg (word_mode, regno, offset, fn);
 -             offset -= fcsr_size;
 -           }
 -         continue;
 -       }
 -
        riscv_save_restore_reg (word_mode, regno, offset, fn);
      }

 diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c b/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
 new file mode 100644
 index 00000000000..d06f05f5c7c
 --- /dev/null
 +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-interrupt-fcsr.c
 @@ -0,0 +1,18 @@
 +/* Verify that fcsr instructions emitted.  */
 +/* { dg-do compile } */
 +/* { dg-require-effective-target hard_float } */
 +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
 +/* { dg-options "-march=rv64gc_xtheadmempair -mtune=thead-c906 -funwind-tables" { target { rv64 } } } */
 +/* { dg-options "-march=rv32gc_xtheadmempair -mtune=thead-c906 -funwind-tables" { target { rv32 } } } */
 +
 +
 +extern int foo (void);
 +
 +void __attribute__ ((interrupt))
 +sub (void)
 +{
 +  foo ();
 +}
 +
 +/* { dg-final { scan-assembler-times "frcsr\t" 1 } } */
 +/* { dg-final { scan-assembler-times "fscsr\t" 1 } } */

 base-commit: e7f4040d9d6ec40c48ada940168885d7dde03af9
 -- 
 2.17.1