| Message ID | 1569395735-18881-1-git-send-email-dmladjenovic@wavecomp.com |
|---|---|
| State | New |
| Headers | show |
| Series | [MIPS] Fix PR target/91769 | expand |
CC: YunQiang Su On 25.09.2019. 09:16, Dragan Mladjenovic wrote: > From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com> > > This fixes the issue by checking that addr's base reg is not part of dest > multiword reg instead just checking the first reg of dest. > > gcc/ChangeLog: > > 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > > PR target/91769 > * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p > instead of REGNO equality check on addr.reg. > > gcc/testsuite/ChangeLog: > > 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > > PR target/91769 > * gcc.target/mips/pr91769.c: New test. > --- > > Hi all, > > Is this OK for trunk? > > The test case is a bit crude, but I guess that is better than having none. > > On top of that, I would like to backport this along with r273174 onto gcc 9 branch. > That should fix BZ91702 and BZ91474 reported against gcc 9.2. > > Tested on mips-mti-linux-gnu. > > Best regards, > Dragan > > gcc/config/mips/mips.c | 2 +- > gcc/testsuite/gcc.target/mips/pr91769.c | 19 +++++++++++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/mips/pr91769.c > > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c > index c682ebd..aa527b4 100644 > --- a/gcc/config/mips/mips.c > +++ b/gcc/config/mips/mips.c > @@ -4862,7 +4862,7 @@ mips_split_move (rtx dest, rtx src, enum mips_split_type split_type, rtx insn_) > { > rtx tmp = XEXP (src, 0); > mips_classify_address (&addr, tmp, GET_MODE (tmp), true); > - if (addr.reg && REGNO (addr.reg) != REGNO (dest)) > + if (addr.reg && !reg_overlap_mentioned_p (dest, addr.reg)) > validate_change (next, &SET_SRC (set), src, false); > } > else > diff --git a/gcc/testsuite/gcc.target/mips/pr91769.c b/gcc/testsuite/gcc.target/mips/pr91769.c > new file mode 100644 > index 0000000..b856183 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/mips/pr91769.c > @@ -0,0 +1,19 @@ > +/* PR target/91769 */ > +/* { dg-do compile } */ > +/* { dg-skip-if "naming registers makes this a code quality test" { *-*-* } { "-O0" "-g" } { "" } } */ > +/* { dg-options "-EL -mgp32 -mhard-float" } */ > + > +NOCOMPRESSION double > +foo (void) > +{ > + register double* pf __asm__ ("$a1"); > + __asm__ __volatile__ ("":"=r"(pf)); > + double f = *pf; > + > + if (f != f) > + f = -f; > + return f; > +} > + > +/* { dg-final { scan-assembler-not "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tldc1\t\\\$.*,0\\(\\\$5\\)" } } */ > +/* { dg-final { scan-assembler "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tmtc1\t\\\$4,\\\$.*\n\tmthc1\t\\\$5,\\\$.*" } } */ >
On 9/25/19 1:16 AM, Dragan Mladjenovic wrote: > From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com> > > This fixes the issue by checking that addr's base reg is not part of dest > multiword reg instead just checking the first reg of dest. > > gcc/ChangeLog: > > 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > > PR target/91769 > * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p > instead of REGNO equality check on addr.reg. > > gcc/testsuite/ChangeLog: > > 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > > PR target/91769 > * gcc.target/mips/pr91769.c: New test. OK. This would seem fine to backport to gcc-9 as well. I don't think gcc-8 had this code. Sorry for introducing the problem (this looks like mine :( jeff
On 01.10.2019. 21:37, Jeff Law wrote: > On 9/25/19 1:16 AM, Dragan Mladjenovic wrote: >> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com> >> >> This fixes the issue by checking that addr's base reg is not part of dest >> multiword reg instead just checking the first reg of dest. >> >> gcc/ChangeLog: >> >> 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> >> >> PR target/91769 >> * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p >> instead of REGNO equality check on addr.reg. >> >> gcc/testsuite/ChangeLog: >> >> 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> >> >> PR target/91769 >> * gcc.target/mips/pr91769.c: New test. > OK. This would seem fine to backport to gcc-9 as well. I don't think > gcc-8 had this code. Committed on trunk as r276525. Committed on gcc-9-branch as r276570 along with the r273174 as r276569. Best regards, Dragan
Hi: The testsuite has a typo in "dg-final scan-assembler", s/mthc1/mtc1/. On Fri, Oct 4, 2019 at 7:21 PM Dragan Mladjenovic <dmladjenovic@wavecomp.com> wrote: > > On 01.10.2019. 21:37, Jeff Law wrote: > > On 9/25/19 1:16 AM, Dragan Mladjenovic wrote: > >> From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com> > >> > >> This fixes the issue by checking that addr's base reg is not part of dest > >> multiword reg instead just checking the first reg of dest. > >> > >> gcc/ChangeLog: > >> > >> 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > >> > >> PR target/91769 > >> * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p > >> instead of REGNO equality check on addr.reg. > >> > >> gcc/testsuite/ChangeLog: > >> > >> 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> > >> > >> PR target/91769 > >> * gcc.target/mips/pr91769.c: New test. > > OK. This would seem fine to backport to gcc-9 as well. I don't think > > gcc-8 had this code. > > Committed on trunk as r276525. > Committed on gcc-9-branch as r276570 along with the r273174 as r276569. > > Best regards, > Dragan >
On 06.10.2019. 08:43, Paul Hua wrote: > Hi: > > The testsuite has a typo in "dg-final scan-assembler", s/mthc1/mtc1/. > Hi, I think I know what is happening here. My testing setup defaults to -mfpxx and yours probably to -mfp32. I should have probably tightened the test up to require R2 isa as well. Does adding isa_rev=2 and -mfpxx to dg-options fix the fallout form this test? I cannot check it right now, but I can send the fix for this tomorrow. Sorry for the inconvenience. Best regards, Dragan
Hi, Thanks for explain that. Add isa_rev=2 and -mfpxx to dg-options fix the fallout. On Sun, Oct 6, 2019 at 8:03 PM Dragan Mladjenovic <dmladjenovic@wavecomp.com> wrote: > > > > On 06.10.2019. 08:43, Paul Hua wrote: > > Hi: > > > > The testsuite has a typo in "dg-final scan-assembler", s/mthc1/mtc1/. > > > > > Hi, > > I think I know what is happening here. My testing setup defaults to > -mfpxx and yours probably to -mfp32. I should have probably tightened > the test up to require R2 isa as well. > Does adding isa_rev=2 and -mfpxx to dg-options fix the fallout form this > test? I cannot check it right now, but I can send the fix for this > tomorrow. Sorry for the inconvenience. > > Best regards, > Dragan
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index c682ebd..aa527b4 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -4862,7 +4862,7 @@ mips_split_move (rtx dest, rtx src, enum mips_split_type split_type, rtx insn_) { rtx tmp = XEXP (src, 0); mips_classify_address (&addr, tmp, GET_MODE (tmp), true); - if (addr.reg && REGNO (addr.reg) != REGNO (dest)) + if (addr.reg && !reg_overlap_mentioned_p (dest, addr.reg)) validate_change (next, &SET_SRC (set), src, false); } else diff --git a/gcc/testsuite/gcc.target/mips/pr91769.c b/gcc/testsuite/gcc.target/mips/pr91769.c new file mode 100644 index 0000000..b856183 --- /dev/null +++ b/gcc/testsuite/gcc.target/mips/pr91769.c @@ -0,0 +1,19 @@ +/* PR target/91769 */ +/* { dg-do compile } */ +/* { dg-skip-if "naming registers makes this a code quality test" { *-*-* } { "-O0" "-g" } { "" } } */ +/* { dg-options "-EL -mgp32 -mhard-float" } */ + +NOCOMPRESSION double +foo (void) +{ + register double* pf __asm__ ("$a1"); + __asm__ __volatile__ ("":"=r"(pf)); + double f = *pf; + + if (f != f) + f = -f; + return f; +} + +/* { dg-final { scan-assembler-not "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tldc1\t\\\$.*,0\\(\\\$5\\)" } } */ +/* { dg-final { scan-assembler "lw\t\\\$4,0\\(\\\$5\\)\n\tlw\t\\\$5,4\\(\\\$5\\)\n\tmtc1\t\\\$4,\\\$.*\n\tmthc1\t\\\$5,\\\$.*" } } */
From: "Dragan Mladjenovic" <dmladjenovic@wavecomp.com> This fixes the issue by checking that addr's base reg is not part of dest multiword reg instead just checking the first reg of dest. gcc/ChangeLog: 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> PR target/91769 * config/mips/mips.c (mips_split_move): Use reg_overlap_mentioned_p instead of REGNO equality check on addr.reg. gcc/testsuite/ChangeLog: 2019-09-25 Dragan Mladjenovic <dmladjenovic@wavecomp.com> PR target/91769 * gcc.target/mips/pr91769.c: New test. --- Hi all, Is this OK for trunk? The test case is a bit crude, but I guess that is better than having none. On top of that, I would like to backport this along with r273174 onto gcc 9 branch. That should fix BZ91702 and BZ91474 reported against gcc 9.2. Tested on mips-mti-linux-gnu. Best regards, Dragan gcc/config/mips/mips.c | 2 +- gcc/testsuite/gcc.target/mips/pr91769.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.target/mips/pr91769.c