[1/1] riscv: thead: Fix ICE when enable XTheadMemPair ISA extension.

The frame related load/store instructions should not been
scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
expression note should should be added to those instructions
to prevent this.
This bug cause ICE during GCC bootstap, and it will also ICE
in the simplified case mempair-4.c, compilation fails with:
during RTL pass: dwarf2
theadmempair-4.c:20:1: internal compiler error: in dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
0xa8c017 dwarf2out_frame_debug_cfa_offset
        ../../../gcc/gcc/dwarf2cfi.cc:1376
0xa8c017 dwarf2out_frame_debug
        ../../../gcc/gcc/dwarf2cfi.cc:2285
0xa8c017 scan_insn_after
        ../../../gcc/gcc/dwarf2cfi.cc:2726
0xa8cc97 scan_trace
        ../../../gcc/gcc/dwarf2cfi.cc:2893
0xa8d84d create_cfi_notes
        ../../../gcc/gcc/dwarf2cfi.cc:2933
0xa8d84d execute_dwarf2_frame
        ../../../gcc/gcc/dwarf2cfi.cc:3309
0xa8d84d execute
        ../../../gcc/gcc/dwarf2cfi.cc:3799

gcc/ChangeLog:

        * config/riscv/thead.cc (th_mempair_save_regs): Add
        REG_FRAME_RELATED_EXPR note for mempair instuctions.

gcc/testsuite/ChangeLog:
        * gcc.target/riscv/xtheadmempair-4.c: New test.
---
 gcc/config/riscv/thead.cc                     |  6 +++--
 .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c

Hi Cooper,

I addressed this in April this year.
It even got an "ok", but nobody pushed it:
  https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html

BR
Christoph

On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <cooper.qu@linux.alibaba.com> wrote:
>
> The frame related load/store instructions should not been
> scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
> expression note should should be added to those instructions
> to prevent this.
> This bug cause ICE during GCC bootstap, and it will also ICE
> in the simplified case mempair-4.c, compilation fails with:
> during RTL pass: dwarf2
> theadmempair-4.c:20:1: internal compiler error: in dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
> 0xa8c017 dwarf2out_frame_debug_cfa_offset
>         ../../../gcc/gcc/dwarf2cfi.cc:1376
> 0xa8c017 dwarf2out_frame_debug
>         ../../../gcc/gcc/dwarf2cfi.cc:2285
> 0xa8c017 scan_insn_after
>         ../../../gcc/gcc/dwarf2cfi.cc:2726
> 0xa8cc97 scan_trace
>         ../../../gcc/gcc/dwarf2cfi.cc:2893
> 0xa8d84d create_cfi_notes
>         ../../../gcc/gcc/dwarf2cfi.cc:2933
> 0xa8d84d execute_dwarf2_frame
>         ../../../gcc/gcc/dwarf2cfi.cc:3309
> 0xa8d84d execute
>         ../../../gcc/gcc/dwarf2cfi.cc:3799
>
> gcc/ChangeLog:
>
>         * config/riscv/thead.cc (th_mempair_save_regs): Add
>         REG_FRAME_RELATED_EXPR note for mempair instuctions.
>
> gcc/testsuite/ChangeLog:
>         * gcc.target/riscv/xtheadmempair-4.c: New test.
> ---
>  gcc/config/riscv/thead.cc                     |  6 +++--
>  .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
>  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
> --- a/gcc/config/riscv/thead.cc
> +++ b/gcc/config/riscv/thead.cc
> @@ -366,10 +366,12 @@ 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;
> -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
> -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
> +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> +  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..d653f056ef4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
> +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv64 } } } */
> +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv32 } } } */
> +
> +void a();
> +void b(char *);
> +void m_fn1(int);
> +int e;
> +
> +int foo(int ee, int f, int g) {
> +  char *h = (char *)__builtin_alloca(1);
> +  b(h);
> +  b("");
> +  int i = ee;
> +  e = g;
> +  m_fn1(f);
> +  a();
> +  e = i;
> +}
> +
> +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64 } } } } */
> +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64 } } } } */
> +
> +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32 } } } } */
> +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32 } } } } */
> --
> 2.17.1
>

Hi Christoph:

Ooops, I thought Philipp will push those patches, does here any other
patches got approved but not committed? I can help to push those
patches tomorrow.

On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
<christoph.muellner@vrull.eu> wrote:
>
> Hi Cooper,
>
> I addressed this in April this year.
> It even got an "ok", but nobody pushed it:
>   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
>
> BR
> Christoph
>
> On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <cooper.qu@linux.alibaba.com> wrote:
> >
> > The frame related load/store instructions should not been
> > scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
> > expression note should should be added to those instructions
> > to prevent this.
> > This bug cause ICE during GCC bootstap, and it will also ICE
> > in the simplified case mempair-4.c, compilation fails with:
> > during RTL pass: dwarf2
> > theadmempair-4.c:20:1: internal compiler error: in dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
> > 0xa8c017 dwarf2out_frame_debug_cfa_offset
> >         ../../../gcc/gcc/dwarf2cfi.cc:1376
> > 0xa8c017 dwarf2out_frame_debug
> >         ../../../gcc/gcc/dwarf2cfi.cc:2285
> > 0xa8c017 scan_insn_after
> >         ../../../gcc/gcc/dwarf2cfi.cc:2726
> > 0xa8cc97 scan_trace
> >         ../../../gcc/gcc/dwarf2cfi.cc:2893
> > 0xa8d84d create_cfi_notes
> >         ../../../gcc/gcc/dwarf2cfi.cc:2933
> > 0xa8d84d execute_dwarf2_frame
> >         ../../../gcc/gcc/dwarf2cfi.cc:3309
> > 0xa8d84d execute
> >         ../../../gcc/gcc/dwarf2cfi.cc:3799
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/thead.cc (th_mempair_save_regs): Add
> >         REG_FRAME_RELATED_EXPR note for mempair instuctions.
> >
> > gcc/testsuite/ChangeLog:
> >         * gcc.target/riscv/xtheadmempair-4.c: New test.
> > ---
> >  gcc/config/riscv/thead.cc                     |  6 +++--
> >  .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
> >  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
> > --- a/gcc/config/riscv/thead.cc
> > +++ b/gcc/config/riscv/thead.cc
> > @@ -366,10 +366,12 @@ 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;
> > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
> > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
> > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> > +  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..d653f056ef4
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
> > +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv64 } } } */
> > +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv32 } } } */
> > +
> > +void a();
> > +void b(char *);
> > +void m_fn1(int);
> > +int e;
> > +
> > +int foo(int ee, int f, int g) {
> > +  char *h = (char *)__builtin_alloca(1);
> > +  b(h);
> > +  b("");
> > +  int i = ee;
> > +  e = g;
> > +  m_fn1(f);
> > +  a();
> > +  e = i;
> > +}
> > +
> > +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64 } } } } */
> > +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64 } } } } */
> > +
> > +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32 } } } } */
> > +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32 } } } } */
> > --
> > 2.17.1
> >

Hi Kito,

I take some of the blame because I have sent a series
that consisted of fixes followed by new features.

You have ack'ed patches 1-9 from the series.
The last two patches (for XTheadMemIdx and XTheadFMemIdx) were
later reviewed by Jeff and need a bit rework and more testing.

If it helps, you can find patches 1-9 rebased and retested here:
  https://github.com/cmuellner/gcc/tree/riscv-thead-improvements

I have also sent out a fix for two failing T-Head tests earlier today:
  https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624049.html
It would be great if you could look at that and push that as well, if it is ok.

Thanks,
Christoph



On Tue, Jul 11, 2023 at 5:51 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> Hi Christoph:
>
> Ooops, I thought Philipp will push those patches, does here any other
> patches got approved but not committed? I can help to push those
> patches tomorrow.
>
> On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
> <christoph.muellner@vrull.eu> wrote:
> >
> > Hi Cooper,
> >
> > I addressed this in April this year.
> > It even got an "ok", but nobody pushed it:
> >   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
> >
> > BR
> > Christoph
> >
> > On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <cooper.qu@linux.alibaba.com> wrote:
> > >
> > > The frame related load/store instructions should not been
> > > scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
> > > expression note should should be added to those instructions
> > > to prevent this.
> > > This bug cause ICE during GCC bootstap, and it will also ICE
> > > in the simplified case mempair-4.c, compilation fails with:
> > > during RTL pass: dwarf2
> > > theadmempair-4.c:20:1: internal compiler error: in dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
> > > 0xa8c017 dwarf2out_frame_debug_cfa_offset
> > >         ../../../gcc/gcc/dwarf2cfi.cc:1376
> > > 0xa8c017 dwarf2out_frame_debug
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2285
> > > 0xa8c017 scan_insn_after
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2726
> > > 0xa8cc97 scan_trace
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2893
> > > 0xa8d84d create_cfi_notes
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2933
> > > 0xa8d84d execute_dwarf2_frame
> > >         ../../../gcc/gcc/dwarf2cfi.cc:3309
> > > 0xa8d84d execute
> > >         ../../../gcc/gcc/dwarf2cfi.cc:3799
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/thead.cc (th_mempair_save_regs): Add
> > >         REG_FRAME_RELATED_EXPR note for mempair instuctions.
> > >
> > > gcc/testsuite/ChangeLog:
> > >         * gcc.target/riscv/xtheadmempair-4.c: New test.
> > > ---
> > >  gcc/config/riscv/thead.cc                     |  6 +++--
> > >  .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
> > >  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
> > > --- a/gcc/config/riscv/thead.cc
> > > +++ b/gcc/config/riscv/thead.cc
> > > @@ -366,10 +366,12 @@ 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;
> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
> > > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> > > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> > > +  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..d653f056ef4
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> > > @@ -0,0 +1,26 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
> > > +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv64 } } } */
> > > +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv32 } } } */
> > > +
> > > +void a();
> > > +void b(char *);
> > > +void m_fn1(int);
> > > +int e;
> > > +
> > > +int foo(int ee, int f, int g) {
> > > +  char *h = (char *)__builtin_alloca(1);
> > > +  b(h);
> > > +  b("");
> > > +  int i = ee;
> > > +  e = g;
> > > +  m_fn1(f);
> > > +  a();
> > > +  e = i;
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64 } } } } */
> > > +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64 } } } } */
> > > +
> > > +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32 } } } } */
> > > +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32 } } } } */
> > > --
> > > 2.17.1
> > >

On Tue, Jul 11, 2023 at 06:02:18PM +0200, Christoph Müllner wrote:
> Hi Kito,
> 
> I take some of the blame because I have sent a series
> that consisted of fixes followed by new features.
> 
> You have ack'ed patches 1-9 from the series.
> The last two patches (for XTheadMemIdx and XTheadFMemIdx) were
> later reviewed by Jeff and need a bit rework and more testing.
> 
> If it helps, you can find patches 1-9 rebased and retested here:
>   https://github.com/cmuellner/gcc/tree/riscv-thead-improvements
> 
> I have also sent out a fix for two failing T-Head tests earlier today:
>   https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624049.html
> It would be great if you could look at that and push that as well, if it is ok.
> 
> Thanks,
> Christoph
> 
> 
> 
> On Tue, Jul 11, 2023 at 5:51 PM Kito Cheng <kito.cheng@sifive.com> wrote:
> >
> > Hi Christoph:
> >
> > Ooops, I thought Philipp will push those patches, does here any other
> > patches got approved but not committed? I can help to push those
> > patches tomorrow.
> >
> > On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
> > <christoph.muellner@vrull.eu> wrote:
> > >
> > > Hi Cooper,
> > >
> > > I addressed this in April this year.
> > > It even got an "ok", but nobody pushed it:
> > >   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
> > >
> > > BR
> > > Christoph
> > >

Hi Christoph and Kito,

That's great that this bug has been resolved. If you merge this patch,
it would be best to also merge it to the gcc-13 branch.


Thanks,
Cooper

Hi Xianmiao:


> Hi Christoph and Kito,
>
> That's great that this bug has been resolved. If you merge this patch,
> it would be best to also merge it to the gcc-13 branch.

Yeah, that sounds reasonable, and the convention for backport is
waiting 1 week to make sure it's stable, so will backport that 1 week
later :)

>
>
> Thanks,
> Cooper

Looks like I missed the OK on this one.
I can pick it up today, unless you Kito already has it in flight?

Thanks,
Philipp.

On Tue, 11 Jul 2023 at 17:51, Kito Cheng <kito.cheng@sifive.com> wrote:

> Hi Christoph:
>
> Ooops, I thought Philipp will push those patches, does here any other
> patches got approved but not committed? I can help to push those
> patches tomorrow.
>
> On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
> <christoph.muellner@vrull.eu> wrote:
> >
> > Hi Cooper,
> >
> > I addressed this in April this year.
> > It even got an "ok", but nobody pushed it:
> >   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
> >
> > BR
> > Christoph
> >
> > On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <cooper.qu@linux.alibaba.com>
> wrote:
> > >
> > > The frame related load/store instructions should not been
> > > scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
> > > expression note should should be added to those instructions
> > > to prevent this.
> > > This bug cause ICE during GCC bootstap, and it will also ICE
> > > in the simplified case mempair-4.c, compilation fails with:
> > > during RTL pass: dwarf2
> > > theadmempair-4.c:20:1: internal compiler error: in
> dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
> > > 0xa8c017 dwarf2out_frame_debug_cfa_offset
> > >         ../../../gcc/gcc/dwarf2cfi.cc:1376
> > > 0xa8c017 dwarf2out_frame_debug
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2285
> > > 0xa8c017 scan_insn_after
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2726
> > > 0xa8cc97 scan_trace
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2893
> > > 0xa8d84d create_cfi_notes
> > >         ../../../gcc/gcc/dwarf2cfi.cc:2933
> > > 0xa8d84d execute_dwarf2_frame
> > >         ../../../gcc/gcc/dwarf2cfi.cc:3309
> > > 0xa8d84d execute
> > >         ../../../gcc/gcc/dwarf2cfi.cc:3799
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/thead.cc (th_mempair_save_regs): Add
> > >         REG_FRAME_RELATED_EXPR note for mempair instuctions.
> > >
> > > gcc/testsuite/ChangeLog:
> > >         * gcc.target/riscv/xtheadmempair-4.c: New test.
> > > ---
> > >  gcc/config/riscv/thead.cc                     |  6 +++--
> > >  .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
> > >  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
> > > --- a/gcc/config/riscv/thead.cc
> > > +++ b/gcc/config/riscv/thead.cc
> > > @@ -366,10 +366,12 @@ 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;
> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
> > > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> > > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> > > +  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..d653f056ef4
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> > > @@ -0,0 +1,26 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" }
> } */
> > > +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g
> -mtune=thead-c906" { target { rv64 } } } */
> > > +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g
> -mtune=thead-c906" { target { rv32 } } } */
> > > +
> > > +void a();
> > > +void b(char *);
> > > +void m_fn1(int);
> > > +int e;
> > > +
> > > +int foo(int ee, int f, int g) {
> > > +  char *h = (char *)__builtin_alloca(1);
> > > +  b(h);
> > > +  b("");
> > > +  int i = ee;
> > > +  e = g;
> > > +  m_fn1(f);
> > > +  a();
> > > +  e = i;
> > > +}
> > > +
> > > +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64 } }
> } } */
> > > +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64 } }
> } } */
> > > +
> > > +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32 } }
> } } */
> > > +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32 } }
> } } */
> > > --
> > > 2.17.1
> > >
>

Yeah, I've applied patches on my local tree and running the testsuite.

On Wed, Jul 12, 2023 at 3:11 PM Philipp Tomsich
<philipp.tomsich@vrull.eu> wrote:
>
> Looks like I missed the OK on this one.
> I can pick it up today, unless you Kito already has it in flight?
>
> Thanks,
> Philipp.
>
> On Tue, 11 Jul 2023 at 17:51, Kito Cheng <kito.cheng@sifive.com> wrote:
>>
>> Hi Christoph:
>>
>> Ooops, I thought Philipp will push those patches, does here any other
>> patches got approved but not committed? I can help to push those
>> patches tomorrow.
>>
>> On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
>> <christoph.muellner@vrull.eu> wrote:
>> >
>> > Hi Cooper,
>> >
>> > I addressed this in April this year.
>> > It even got an "ok", but nobody pushed it:
>> >   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
>> >
>> > BR
>> > Christoph
>> >
>> > On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <cooper.qu@linux.alibaba.com> wrote:
>> > >
>> > > The frame related load/store instructions should not been
>> > > scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
>> > > expression note should should be added to those instructions
>> > > to prevent this.
>> > > This bug cause ICE during GCC bootstap, and it will also ICE
>> > > in the simplified case mempair-4.c, compilation fails with:
>> > > during RTL pass: dwarf2
>> > > theadmempair-4.c:20:1: internal compiler error: in dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
>> > > 0xa8c017 dwarf2out_frame_debug_cfa_offset
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:1376
>> > > 0xa8c017 dwarf2out_frame_debug
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:2285
>> > > 0xa8c017 scan_insn_after
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:2726
>> > > 0xa8cc97 scan_trace
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:2893
>> > > 0xa8d84d create_cfi_notes
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:2933
>> > > 0xa8d84d execute_dwarf2_frame
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:3309
>> > > 0xa8d84d execute
>> > >         ../../../gcc/gcc/dwarf2cfi.cc:3799
>> > >
>> > > gcc/ChangeLog:
>> > >
>> > >         * config/riscv/thead.cc (th_mempair_save_regs): Add
>> > >         REG_FRAME_RELATED_EXPR note for mempair instuctions.
>> > >
>> > > gcc/testsuite/ChangeLog:
>> > >         * gcc.target/riscv/xtheadmempair-4.c: New test.
>> > > ---
>> > >  gcc/config/riscv/thead.cc                     |  6 +++--
>> > >  .../gcc.target/riscv/xtheadmempair-4.c        | 26 +++++++++++++++++++
>> > >  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
>> > > --- a/gcc/config/riscv/thead.cc
>> > > +++ b/gcc/config/riscv/thead.cc
>> > > @@ -366,10 +366,12 @@ 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;
>> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
>> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
>> > > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
>> > > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
>> > > +  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..d653f056ef4
>> > > --- /dev/null
>> > > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
>> > > @@ -0,0 +1,26 @@
>> > > +/* { dg-do compile } */
>> > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os" "-flto" } } */
>> > > +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv64 } } } */
>> > > +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g -mtune=thead-c906" { target { rv32 } } } */
>> > > +
>> > > +void a();
>> > > +void b(char *);
>> > > +void m_fn1(int);
>> > > +int e;
>> > > +
>> > > +int foo(int ee, int f, int g) {
>> > > +  char *h = (char *)__builtin_alloca(1);
>> > > +  b(h);
>> > > +  b("");
>> > > +  int i = ee;
>> > > +  e = g;
>> > > +  m_fn1(f);
>> > > +  a();
>> > > +  e = i;
>> > > +}
>> > > +
>> > > +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64 } } } } */
>> > > +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64 } } } } */
>> > > +
>> > > +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32 } } } } */
>> > > +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32 } } } } */
>> > > --
>> > > 2.17.1
>> > >

Awesome, thanks!

On Wed, 12 Jul 2023 at 09:18, Kito Cheng <kito.cheng@sifive.com> wrote:

> Yeah, I've applied patches on my local tree and running the testsuite.
>
> On Wed, Jul 12, 2023 at 3:11 PM Philipp Tomsich
> <philipp.tomsich@vrull.eu> wrote:
> >
> > Looks like I missed the OK on this one.
> > I can pick it up today, unless you Kito already has it in flight?
> >
> > Thanks,
> > Philipp.
> >
> > On Tue, 11 Jul 2023 at 17:51, Kito Cheng <kito.cheng@sifive.com> wrote:
> >>
> >> Hi Christoph:
> >>
> >> Ooops, I thought Philipp will push those patches, does here any other
> >> patches got approved but not committed? I can help to push those
> >> patches tomorrow.
> >>
> >> On Tue, Jul 11, 2023 at 11:42 PM Christoph Müllner
> >> <christoph.muellner@vrull.eu> wrote:
> >> >
> >> > Hi Cooper,
> >> >
> >> > I addressed this in April this year.
> >> > It even got an "ok", but nobody pushed it:
> >> >   https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616972.html
> >> >
> >> > BR
> >> > Christoph
> >> >
> >> > On Tue, Jul 11, 2023 at 5:39 PM Xianmiao Qu <
> cooper.qu@linux.alibaba.com> wrote:
> >> > >
> >> > > The frame related load/store instructions should not been
> >> > > scheduled bewteen echo other, and the REG_FRAME_RELATED_EXPR
> >> > > expression note should should be added to those instructions
> >> > > to prevent this.
> >> > > This bug cause ICE during GCC bootstap, and it will also ICE
> >> > > in the simplified case mempair-4.c, compilation fails with:
> >> > > during RTL pass: dwarf2
> >> > > theadmempair-4.c:20:1: internal compiler error: in
> dwarf2out_frame_debug_cfa_offset, at dwarf2cfi.cc:1376
> >> > > 0xa8c017 dwarf2out_frame_debug_cfa_offset
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:1376
> >> > > 0xa8c017 dwarf2out_frame_debug
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:2285
> >> > > 0xa8c017 scan_insn_after
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:2726
> >> > > 0xa8cc97 scan_trace
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:2893
> >> > > 0xa8d84d create_cfi_notes
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:2933
> >> > > 0xa8d84d execute_dwarf2_frame
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:3309
> >> > > 0xa8d84d execute
> >> > >         ../../../gcc/gcc/dwarf2cfi.cc:3799
> >> > >
> >> > > gcc/ChangeLog:
> >> > >
> >> > >         * config/riscv/thead.cc (th_mempair_save_regs): Add
> >> > >         REG_FRAME_RELATED_EXPR note for mempair instuctions.
> >> > >
> >> > > gcc/testsuite/ChangeLog:
> >> > >         * gcc.target/riscv/xtheadmempair-4.c: New test.
> >> > > ---
> >> > >  gcc/config/riscv/thead.cc                     |  6 +++--
> >> > >  .../gcc.target/riscv/xtheadmempair-4.c        | 26
> +++++++++++++++++++
> >> > >  2 files changed, 30 insertions(+), 2 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 75203805310..2df709226f9 100644
> >> > > --- a/gcc/config/riscv/thead.cc
> >> > > +++ b/gcc/config/riscv/thead.cc
> >> > > @@ -366,10 +366,12 @@ 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;
> >> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set1));
> >> > > -  add_reg_note (insn, REG_CFA_OFFSET, copy_rtx (set2));
> >> > > +  XVECEXP (dwarf, 0, 0) = copy_rtx (set1);
> >> > > +  XVECEXP (dwarf, 0, 1) = copy_rtx (set2);
> >> > > +  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..d653f056ef4
> >> > > --- /dev/null
> >> > > +++ b/gcc/testsuite/gcc.target/riscv/xtheadmempair-4.c
> >> > > @@ -0,0 +1,26 @@
> >> > > +/* { dg-do compile } */
> >> > > +/* { dg-skip-if "" { *-*-* } { "-O0" "-O1" "-g" "-Oz" "-Os"
> "-flto" } } */
> >> > > +/* { dg-options "-march=rv64gc_xtheadmempair -O2 -g
> -mtune=thead-c906" { target { rv64 } } } */
> >> > > +/* { dg-options "-march=rv32gc_xtheadmempair -O2 -g
> -mtune=thead-c906" { target { rv32 } } } */
> >> > > +
> >> > > +void a();
> >> > > +void b(char *);
> >> > > +void m_fn1(int);
> >> > > +int e;
> >> > > +
> >> > > +int foo(int ee, int f, int g) {
> >> > > +  char *h = (char *)__builtin_alloca(1);
> >> > > +  b(h);
> >> > > +  b("");
> >> > > +  int i = ee;
> >> > > +  e = g;
> >> > > +  m_fn1(f);
> >> > > +  a();
> >> > > +  e = i;
> >> > > +}
> >> > > +
> >> > > +/* { dg-final { scan-assembler-times "th.ldd\t" 3 { target { rv64
> } } } } */
> >> > > +/* { dg-final { scan-assembler-times "th.sdd\t" 3 { target { rv64
> } } } } */
> >> > > +
> >> > > +/* { dg-final { scan-assembler-times "th.lwd\t" 3 { target { rv32
> } } } } */
> >> > > +/* { dg-final { scan-assembler-times "th.swd\t" 3 { target { rv32
> } } } } */
> >> > > --
> >> > > 2.17.1
> >> > >
>