diff mbox

combine: distribute_notes again (PR69567, PR64682)

Message ID 9f15493d0cbf1a52a475998ffa22f1fc26f5e275.1454366721.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Feb. 2, 2016, 11:32 a.m. UTC 
As it happens the patch I did over a year ago for PR64682 isn't quite
correct.  This is PR69567.  This fixes it.

Tested on the separate testcases; also did bootstrap + testsuite on
powerpc64-linux and x86_64-linux.  I'll commit this tomorrow or so if
no one sees something wrong with it.


Segher


2016-02-02  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/64682
	PR rtl-optimization/69567
	* combine.c (distribute_notes) <REG_DEAD>: Place the death note
	before I2 only if the register is both used and set in I2.

---
 gcc/combine.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Segher Boessenkool Feb. 4, 2016, 11:28 p.m. UTC | #1 
On Tue, Feb 02, 2016 at 11:32:08AM +0000, Segher Boessenkool wrote:
> As it happens the patch I did over a year ago for PR64682 isn't quite
> correct.  This is PR69567.  This fixes it.
> 
> Tested on the separate testcases; also did bootstrap + testsuite on
> powerpc64-linux and x86_64-linux.  I'll commit this tomorrow or so if
> no one sees something wrong with it.
> 
> 
> 2016-02-02  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR rtl-optimization/64682
> 	PR rtl-optimization/69567
> 	* combine.c (distribute_notes) <REG_DEAD>: Place the death note
> 	before I2 only if the register is both used and set in I2.

Committed to mainline and 5 now.


Segher
H.J. Lu Feb. 6, 2016, 5:54 p.m. UTC | #2 
On Thu, Feb 4, 2016 at 3:28 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Tue, Feb 02, 2016 at 11:32:08AM +0000, Segher Boessenkool wrote:
>> As it happens the patch I did over a year ago for PR64682 isn't quite
>> correct.  This is PR69567.  This fixes it.
>>
>> Tested on the separate testcases; also did bootstrap + testsuite on
>> powerpc64-linux and x86_64-linux.  I'll commit this tomorrow or so if
>> no one sees something wrong with it.
>>
>>
>> 2016-02-02  Segher Boessenkool  <segher@kernel.crashing.org>
>>
>>       PR rtl-optimization/64682
>>       PR rtl-optimization/69567
>>       * combine.c (distribute_notes) <REG_DEAD>: Place the death note
>>       before I2 only if the register is both used and set in I2.
>
> Committed to mainline and 5 now.
>

This caused:

FAIL: gcc.c-torture/execute/pr64682.c   -O2  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto
-fno-use-linker-plugin -flto-partition=none  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
-funroll-loops  execution test
FAIL: gcc.c-torture/execute/pr64682.c   -O3 -g  execution test

on x86 on GCC 5 branch.  Trunk is OK.  Did you miss a backport from
trunk?
Segher Boessenkool Feb. 7, 2016, 12:51 a.m. UTC | #3 
On Sat, Feb 06, 2016 at 09:54:25AM -0800, H.J. Lu wrote:
> On Thu, Feb 4, 2016 at 3:28 PM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Tue, Feb 02, 2016 at 11:32:08AM +0000, Segher Boessenkool wrote:
> >> As it happens the patch I did over a year ago for PR64682 isn't quite
> >> correct.  This is PR69567.  This fixes it.
> >>
> >> Tested on the separate testcases; also did bootstrap + testsuite on
> >> powerpc64-linux and x86_64-linux.  I'll commit this tomorrow or so if
> >> no one sees something wrong with it.
> >>
> >>
> >> 2016-02-02  Segher Boessenkool  <segher@kernel.crashing.org>
> >>
> >>       PR rtl-optimization/64682
> >>       PR rtl-optimization/69567
> >>       * combine.c (distribute_notes) <REG_DEAD>: Place the death note
> >>       before I2 only if the register is both used and set in I2.
> >
> > Committed to mainline and 5 now.
> >
> 
> This caused:
> 
> FAIL: gcc.c-torture/execute/pr64682.c   -O2  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto
> -fno-use-linker-plugin -flto-partition=none  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
> -funroll-loops  execution test
> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -g  execution test
> 
> on x86 on GCC 5 branch.  Trunk is OK.  Did you miss a backport from
> trunk?

I tested it on x86_64-linux.  What does "x86" mean, exactly?

Note -O2 and -O3 do not exhibit the fail condition on trunk on the 64682
testcase (but -O1 does).


Segher
H.J. Lu Feb. 7, 2016, 1:20 a.m. UTC | #4 
On Sat, Feb 6, 2016 at 4:51 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Sat, Feb 06, 2016 at 09:54:25AM -0800, H.J. Lu wrote:
>> On Thu, Feb 4, 2016 at 3:28 PM, Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> > On Tue, Feb 02, 2016 at 11:32:08AM +0000, Segher Boessenkool wrote:
>> >> As it happens the patch I did over a year ago for PR64682 isn't quite
>> >> correct.  This is PR69567.  This fixes it.
>> >>
>> >> Tested on the separate testcases; also did bootstrap + testsuite on
>> >> powerpc64-linux and x86_64-linux.  I'll commit this tomorrow or so if
>> >> no one sees something wrong with it.
>> >>
>> >>
>> >> 2016-02-02  Segher Boessenkool  <segher@kernel.crashing.org>
>> >>
>> >>       PR rtl-optimization/64682
>> >>       PR rtl-optimization/69567
>> >>       * combine.c (distribute_notes) <REG_DEAD>: Place the death note
>> >>       before I2 only if the register is both used and set in I2.
>> >
>> > Committed to mainline and 5 now.
>> >
>>
>> This caused:
>>
>> FAIL: gcc.c-torture/execute/pr64682.c   -O2  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto
>> -fno-use-linker-plugin -flto-partition=none  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O2 -flto -fuse-linker-plugin
>> -fno-fat-lto-objects  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
>> -funroll-all-loops -finline-functions  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -fomit-frame-pointer
>> -funroll-loops  execution test
>> FAIL: gcc.c-torture/execute/pr64682.c   -O3 -g  execution test
>>
>> on x86 on GCC 5 branch.  Trunk is OK.  Did you miss a backport from
>> trunk?
>
> I tested it on x86_64-linux.  What does "x86" mean, exactly?
>
> Note -O2 and -O3 do not exhibit the fail condition on trunk on the 64682
> testcase (but -O1 does).
>

By x86, I mean ia32, x86-64 and x32:

https://gcc.gnu.org/ml/gcc-testresults/2016-02/msg00475.html
https://gcc.gnu.org/ml/gcc-testresults/2016-02/msg00645.html
https://gcc.gnu.org/ml/gcc-testresults/2016-02/msg00503.html

It only happens on gcc-5-branch, not on trunk:

/export/build/gnu/gcc-5/build-x86_64-linux/gcc/
/export/gnu/import/git/sources/gcc-release/gcc/testsuite/gcc.c-torture/execute/pr64682.c
-fno-diagnostics-show-caret -fdiagnostics-color=never -O2 -w -lm -o
./pr64682.exe
[hjl@gnu-skl-1 testsuite]$ gdb ./pr64682.exe
GNU gdb (GDB) Fedora 7.10.1-30.fc23
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./pr64682.exe...(no debugging symbols found)...done.
(gdb) r
Starting program:
/export/build/gnu/gcc-5/build-x86_64-linux/gcc/testsuite/pr64682.exe
Missing separate debuginfos, use: dnf debuginfo-install
glibc-2.22-7.0.fc23.x86_64

Program received signal SIGABRT, Aborted.
0x00007ffff774c3b8 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff774c3b8 in raise () from /lib64/libc.so.6
#1  0x00007ffff774dfba in abort () from /lib64/libc.so.6
#2  0x00000000004005dd in foo ()
#3  0x00000000004004b6 in main ()
(gdb)
diff mbox

Patch

diff --git a/gcc/combine.c b/gcc/combine.c
index ad79c44..7251078 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -13888,6 +13888,7 @@  distribute_notes (rtx notes, rtx_insn *from_insn, rtx_insn *i3, rtx_insn *i2,
 	    tem_insn = from_insn;
 	  else
 	    {
+	      tem_insn = i3;
 	      if (from_insn
 		  && CALL_P (from_insn)
 		  && find_reg_fusage (from_insn, USE, XEXP (note, 0)))
@@ -13896,7 +13897,14 @@  distribute_notes (rtx notes, rtx_insn *from_insn, rtx_insn *i3, rtx_insn *i2,
 		place = i3;
 	      else if (i2 != 0 && next_nonnote_nondebug_insn (i2) == i3
 		       && reg_referenced_p (XEXP (note, 0), PATTERN (i2)))
-		place = i2;
+		{
+		  place = i2;
+		  /* If the new I2 sets the same register that is marked dead
+		     in the note, the note now should not be put on I2, as the
+		     note refers to a previous incarnation of the reg.  */
+		  if (reg_set_p (XEXP (note, 0), PATTERN (i2)))
+		    tem_insn = i2;
+		}
 	      else if ((rtx_equal_p (XEXP (note, 0), elim_i2)
 			&& !(i2mod
 			     && reg_overlap_mentioned_p (XEXP (note, 0),
@@ -13904,12 +13912,6 @@  distribute_notes (rtx notes, rtx_insn *from_insn, rtx_insn *i3, rtx_insn *i2,
 		       || rtx_equal_p (XEXP (note, 0), elim_i1)
 		       || rtx_equal_p (XEXP (note, 0), elim_i0))
 		break;
-	      tem_insn = i3;
-	      /* If the new I2 sets the same register that is marked dead
-		 in the note, the note now should not be put on I2, as the
-		 note refers to a previous incarnation of the reg.  */
-	      if (i2 != 0 && reg_set_p (XEXP (note, 0), PATTERN (i2)))
-		tem_insn = i2;
 	    }
 
 	  if (place == 0)