diff mbox series

RISC-V: THead: Fix missing CFI directives for th.sdd in prologue.

Message ID 20231004074906.15805-1-cooper.qu@linux.alibaba.com
State Unresolved
Headers
Series RISC-V: THead: Fix missing CFI directives for th.sdd in prologue. |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

瞿仙淼 Oct. 4, 2023, 7:49 a.m. UTC 
  From: quxm <xianmiao.qxm@alibaba-inc.com>

When generating CFI directives for the store-pair instruction,
if we add two parallel REG_FRAME_RELATED_EXPR expr_lists like
  (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (plus:DI (reg/f:DI 2 sp)
    (const_int 8 [0x8])) [1  S8 A64])
    (reg:DI 1 ra))
  (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (reg/f:DI 2 sp) [1  S8 A64])
    (reg:DI 8 s0))
only the first expr_list will be recognized by dwarf2out_frame_debug
funciton. So, here we generate a SEQUENCE expression of REG_FRAME_RELATED_EXPR,
which includes two sub-expressions of RTX_FRAME_RELATED_P. Then the
dwarf2out_frame_debug_expr function will iterate through all the sub-expressions
and generate the corresponding CFI directives.

gcc/
	* config/riscv/thead.cc (th_mempair_save_regs): Fix missing CFI
	directives for store-pair instruction.

gcc/testsuite/
	* gcc.target/riscv/xtheadmempair-4.c: New test.
---
 gcc/config/riscv/thead.cc                     | 11 +++----
 .../gcc.target/riscv/xtheadmempair-4.c        | 29 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c

Comments

瞿仙淼 Oct. 4, 2023, 8:31 a.m. UTC | #1 
On 10/4/23 15:49, Xianmiao Qu wrote:
> 
> From: quxm <xianmiao.qxm@alibaba-inc.com>
> 
I'm sorry for posting the wrong username and email. If someone helps me merge the code later, please delete this line for me (Just use the username and email from the email I am currently sending, Xianmiao Qu <cooper.qu@linux.alibaba.com>).
Thanks,
Xianmiao
Christoph Müllner Oct. 4, 2023, 3:04 p.m. UTC | #2 
On Wed, Oct 4, 2023 at 9:49 AM Xianmiao Qu <cooper.qu@linux.alibaba.com> wrote:
>
> From: quxm <xianmiao.qxm@alibaba-inc.com>
>
> When generating CFI directives for the store-pair instruction,
> if we add two parallel REG_FRAME_RELATED_EXPR expr_lists like
>   (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (plus:DI (reg/f:DI 2 sp)
>     (const_int 8 [0x8])) [1  S8 A64])
>     (reg:DI 1 ra))
>   (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (reg/f:DI 2 sp) [1  S8 A64])
>     (reg:DI 8 s0))
> only the first expr_list will be recognized by dwarf2out_frame_debug
> funciton. So, here we generate a SEQUENCE expression of REG_FRAME_RELATED_EXPR,
> which includes two sub-expressions of RTX_FRAME_RELATED_P. Then the
> dwarf2out_frame_debug_expr function will iterate through all the sub-expressions
> and generate the corresponding CFI directives.
>
> gcc/
>         * config/riscv/thead.cc (th_mempair_save_regs): Fix missing CFI
>         directives for store-pair instruction.
>
> gcc/testsuite/
>         * gcc.target/riscv/xtheadmempair-4.c: New test.

LGTM, I've also tested it.

Reviewed-by: Christoph Müllner <christoph.muellner@vrull.eu>
Tested-by: Christoph Müllner <christoph.muellner@vrull.eu>

Thanks!

> ---
>  gcc/config/riscv/thead.cc                     | 11 +++----
>  .../gcc.target/riscv/xtheadmempair-4.c        | 29 +++++++++++++++++++
>  2 files changed, 35 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
>
> diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc
> index 507c912bc39..be0cd7c1276 100644
> --- a/gcc/config/riscv/thead.cc
> +++ b/gcc/config/riscv/thead.cc
> @@ -366,14 +366,15 @@ th_mempair_save_regs (rtx operands[4])
>  {
>    rtx set1 = gen_rtx_SET (operands[0], operands[1]);
>    rtx set2 = gen_rtx_SET (operands[2], operands[3]);
> +  rtx dwarf = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
>    rtx insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, set1, set2)));
>    RTX_FRAME_RELATED_P (insn) = 1;
>
> -  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
> -                                     copy_rtx (set1), REG_NOTES (insn));
> -
> -  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
> -                                     copy_rtx (set2), REG_NOTES (insn));
> +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> +  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 0)) = 1;
> +  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 1)) = 1;
> +  add_reg_note (insn, REG_FRAME_RELATED_EXPR, dwarf);
>  }
>
>  /* Similar like riscv_restore_reg, but restores two registers from memory
> diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> new file mode 100644
> index 00000000000..9aef4e15f8d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { 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 void bar (void);
> +
> +void foo (void)
> +{
> +  asm volatile (";my clobber list"
> +               : : : "s0");
> +  bar ();
> +  asm volatile (";my clobber list"
> +               : : : "s0");
> +}
> +
> +/* { dg-final { scan-assembler-times "th.sdd\t" 1 { target { rv64 } } } } */
> +/* { dg-final { scan-assembler ".cfi_offset 8, -16" { target { rv64 } } } } */
> +/* { dg-final { scan-assembler ".cfi_offset 1, -8" { target { rv64 } } } } */
> +
> +/* { dg-final { scan-assembler-times "th.swd\t" 1 { target { rv32 } } } } */
> +/* { dg-final { scan-assembler ".cfi_offset 8, -8" { target { rv32 } } } } */
> +/* { dg-final { scan-assembler ".cfi_offset 1, -4" { target { rv32 } } } } */
> +
> +/* { dg-final { scan-assembler ".cfi_restore 1" } } */
> +/* { dg-final { scan-assembler ".cfi_restore 8" } } */
> +
> +/* { dg-final { scan-assembler-times "th.ldd\t" 1 { target { rv64 } } } } */
> +/* { dg-final { scan-assembler-times "th.lwd\t" 1 { target { rv32 } } } } */
> --
> 2.17.1
>
Kito Cheng Oct. 4, 2023, 3:08 p.m. UTC | #3 
LGTM, will commit if nobody commit that tomorrow :p

Christoph Müllner <christoph.muellner@vrull.eu>於 2023年10月4日 週三，23:04寫道：

> On Wed, Oct 4, 2023 at 9:49 AM Xianmiao Qu <cooper.qu@linux.alibaba.com>
> wrote:
> >
> > From: quxm <xianmiao.qxm@alibaba-inc.com>
> >
> > When generating CFI directives for the store-pair instruction,
> > if we add two parallel REG_FRAME_RELATED_EXPR expr_lists like
> >   (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (plus:DI (reg/f:DI 2
> sp)
> >     (const_int 8 [0x8])) [1  S8 A64])
> >     (reg:DI 1 ra))
> >   (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (reg/f:DI 2 sp) [1
> S8 A64])
> >     (reg:DI 8 s0))
> > only the first expr_list will be recognized by dwarf2out_frame_debug
> > funciton. So, here we generate a SEQUENCE expression of
> REG_FRAME_RELATED_EXPR,
> > which includes two sub-expressions of RTX_FRAME_RELATED_P. Then the
> > dwarf2out_frame_debug_expr function will iterate through all the
> sub-expressions
> > and generate the corresponding CFI directives.
> >
> > gcc/
> >         * config/riscv/thead.cc (th_mempair_save_regs): Fix missing CFI
> >         directives for store-pair instruction.
> >
> > gcc/testsuite/
> >         * gcc.target/riscv/xtheadmempair-4.c: New test.
>
> LGTM, I've also tested it.
>
> Reviewed-by: Christoph Müllner <christoph.muellner@vrull.eu>
> Tested-by: Christoph Müllner <christoph.muellner@vrull.eu>
>
> Thanks!
>
> > ---
> >  gcc/config/riscv/thead.cc                     | 11 +++----
> >  .../gcc.target/riscv/xtheadmempair-4.c        | 29 +++++++++++++++++++
> >  2 files changed, 35 insertions(+), 5 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> >
> > diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc
> > index 507c912bc39..be0cd7c1276 100644
> > --- a/gcc/config/riscv/thead.cc
> > +++ b/gcc/config/riscv/thead.cc
> > @@ -366,14 +366,15 @@ th_mempair_save_regs (rtx operands[4])
> >  {
> >    rtx set1 = gen_rtx_SET (operands[0], operands[1]);
> >    rtx set2 = gen_rtx_SET (operands[2], operands[3]);
> > +  rtx dwarf = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
> >    rtx insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, set1,
> set2)));
> >    RTX_FRAME_RELATED_P (insn) = 1;
> >
> > -  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
> > -                                     copy_rtx (set1), REG_NOTES (insn));
> > -
> > -  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
> > -                                     copy_rtx (set2), REG_NOTES (insn));
> > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> > +  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 0)) = 1;
> > +  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 1)) = 1;
> > +  add_reg_note (insn, REG_FRAME_RELATED_EXPR, dwarf);
> >  }
> >
> >  /* Similar like riscv_restore_reg, but restores two registers from
> memory
> > diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> > new file mode 100644
> > index 00000000000..9aef4e15f8d
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> > @@ -0,0 +1,29 @@
> > +/* { dg-do compile } */
> > +/* { 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 void bar (void);
> > +
> > +void foo (void)
> > +{
> > +  asm volatile (";my clobber list"
> > +               : : : "s0");
> > +  bar ();
> > +  asm volatile (";my clobber list"
> > +               : : : "s0");
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "th.sdd\t" 1 { target { rv64 } } }
> } */
> > +/* { dg-final { scan-assembler ".cfi_offset 8, -16" { target { rv64 } }
> } } */
> > +/* { dg-final { scan-assembler ".cfi_offset 1, -8" { target { rv64 } }
> } } */
> > +
> > +/* { dg-final { scan-assembler-times "th.swd\t" 1 { target { rv32 } } }
> } */
> > +/* { dg-final { scan-assembler ".cfi_offset 8, -8" { target { rv32 } }
> } } */
> > +/* { dg-final { scan-assembler ".cfi_offset 1, -4" { target { rv32 } }
> } } */
> > +
> > +/* { dg-final { scan-assembler ".cfi_restore 1" } } */
> > +/* { dg-final { scan-assembler ".cfi_restore 8" } } */
> > +
> > +/* { dg-final { scan-assembler-times "th.ldd\t" 1 { target { rv64 } } }
> } */
> > +/* { dg-final { scan-assembler-times "th.lwd\t" 1 { target { rv32 } } }
> } */
> > --
> > 2.17.1
> >
>
Jeff Law Oct. 9, 2023, 1:25 p.m. UTC | #4 
On 10/4/23 01:49, Xianmiao Qu wrote:
> From: quxm <xianmiao.qxm@alibaba-inc.com>
> 
> When generating CFI directives for the store-pair instruction,
> if we add two parallel REG_FRAME_RELATED_EXPR expr_lists like
>    (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (plus:DI (reg/f:DI 2 sp)
>      (const_int 8 [0x8])) [1  S8 A64])
>      (reg:DI 1 ra))
>    (expr_list:REG_FRAME_RELATED_EXPR (set (mem/c:DI (reg/f:DI 2 sp) [1  S8 A64])
>      (reg:DI 8 s0))
> only the first expr_list will be recognized by dwarf2out_frame_debug
> funciton. So, here we generate a SEQUENCE expression of REG_FRAME_RELATED_EXPR,
> which includes two sub-expressions of RTX_FRAME_RELATED_P. Then the
> dwarf2out_frame_debug_expr function will iterate through all the sub-expressions
> and generate the corresponding CFI directives.
> 
> gcc/
> 	* config/riscv/thead.cc (th_mempair_save_regs): Fix missing CFI
> 	directives for store-pair instruction.
> 
> gcc/testsuite/
> 	* gcc.target/riscv/xtheadmempair-4.c: New test.
THanks.  I pushed this to the trunk.
jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/thead.cc b/gcc/config/riscv/thead.cc
index 507c912bc39..be0cd7c1276 100644
--- a/gcc/config/riscv/thead.cc
+++ b/gcc/config/riscv/thead.cc
@@ -366,14 +366,15 @@  th_mempair_save_regs (rtx operands[4])
 {
   rtx set1 = gen_rtx_SET (operands[0], operands[1]);
   rtx set2 = gen_rtx_SET (operands[2], operands[3]);
+  rtx dwarf = gen_rtx_SEQUENCE (VOIDmode, rtvec_alloc (2));
   rtx insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, set1, set2)));
   RTX_FRAME_RELATED_P (insn) = 1;
 
-  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
-				      copy_rtx (set1), REG_NOTES (insn));
-
-  REG_NOTES (insn) = alloc_EXPR_LIST (REG_FRAME_RELATED_EXPR,
-				      copy_rtx (set2), REG_NOTES (insn));
+  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
+  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
+  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 0)) = 1;
+  RTX_FRAME_RELATED_P (XVECEXP (dwarf, 0, 1)) = 1;
+  add_reg_note (insn, REG_FRAME_RELATED_EXPR, dwarf);
 }
 
 /* Similar like riscv_restore_reg, but restores two registers from memory
diff --git a/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
new file mode 100644
index 00000000000..9aef4e15f8d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
@@ -0,0 +1,29 @@ 
+/* { dg-do compile } */
+/* { 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 void bar (void);
+
+void foo (void)
+{
+  asm volatile (";my clobber list"
+		: : : "s0");
+  bar ();
+  asm volatile (";my clobber list"
+		: : : "s0");
+}
+
+/* { dg-final { scan-assembler-times "th.sdd\t" 1 { target { rv64 } } } } */
+/* { dg-final { scan-assembler ".cfi_offset 8, -16" { target { rv64 } } } } */
+/* { dg-final { scan-assembler ".cfi_offset 1, -8" { target { rv64 } } } } */
+
+/* { dg-final { scan-assembler-times "th.swd\t" 1 { target { rv32 } } } } */
+/* { dg-final { scan-assembler ".cfi_offset 8, -8" { target { rv32 } } } } */
+/* { dg-final { scan-assembler ".cfi_offset 1, -4" { target { rv32 } } } } */
+
+/* { dg-final { scan-assembler ".cfi_restore 1" } } */
+/* { dg-final { scan-assembler ".cfi_restore 8" } } */
+
+/* { dg-final { scan-assembler-times "th.ldd\t" 1 { target { rv64 } } } } */
+/* { dg-final { scan-assembler-times "th.lwd\t" 1 { target { rv32 } } } } */