diff mbox

[rtl-optimization/49847] Fix cse to handle cc0 setter and cc0 user in different blocks

Message ID 530F8BC9.1020505@redhat.com
State New
Headers show

Commit Message

Jeff Law Feb. 27, 2014, 7:02 p.m. UTC
As discussed in 49847, a few years ago GCC was changed to add EH edges 
for exceptions that might arise from a floating point comparison.  That 
change made it possible for a cc0-setter and cc0-user to end up in 
different blocks, separated by a NOTE.

After discussing reverting the change, duplicating the cc0-setter and a 
couple other ideas, eventually Richi and myself settled on relaxing the 
long standing restrictions on the relative locations of the cc0 setter 
and cc0 user.

We agreed to update the documentation and fault in fixes due to low 
priority of cc0 targets (and doubly so since this really only affects 
cc0 targets with used with non-call-exceptions).

This patch updates the documentation and fixes the only known failure 
due to this change in cse.c with a patch from Mikael.

Tested with a cross compiler.  I'll probably fire off a m68k bootstrap 
for good measure, but it'll be several days before that completes.

Installed on the trunk.

Jeff

Comments

H.J. Lu Feb. 27, 2014, 7:50 p.m. UTC | #1
On Thu, Feb 27, 2014 at 11:02 AM, Jeff Law <law@redhat.com> wrote:
>
> As discussed in 49847, a few years ago GCC was changed to add EH edges for
> exceptions that might arise from a floating point comparison.  That change
> made it possible for a cc0-setter and cc0-user to end up in different
> blocks, separated by a NOTE.
>
> After discussing reverting the change, duplicating the cc0-setter and a
> couple other ideas, eventually Richi and myself settled on relaxing the long
> standing restrictions on the relative locations of the cc0 setter and cc0
> user.
>
> We agreed to update the documentation and fault in fixes due to low priority
> of cc0 targets (and doubly so since this really only affects cc0 targets
> with used with non-call-exceptions).
>
> This patch updates the documentation and fixes the only known failure due to
> this change in cse.c with a patch from Mikael.
>
> Tested with a cross compiler.  I'll probably fire off a m68k bootstrap for
> good measure, but it'll be several days before that completes.
>
> Installed on the trunk.
>
> Jeff
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 8a78716..a20cee3 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2014-02-27  Mikael Pettersson  <mikpe@it.uu.se>
> +           Jeff Law  <law@redhat.com>
> +
> +       PR rtl-optimization/49847
> +       * cse.c (fold_rtx) Handle case where cc0 setter and cc0 user
> +       are in different blocks.
> +       * doc/tm.texi (Condition Code Status): Update documention for
> +       relative locations of cc0-setter and cc0-user.
> +

This breaks bootstrap:

You should edit ../../src-trunk/gcc/doc/tm.texi.in rather than
../../src-trunk/gcc/doc/tm.texi .
make[6]: *** [s-tm-texi] Error 1
make[6]: *** Waiting for unfinished jobs....
Jeff Law Feb. 27, 2014, 9:50 p.m. UTC | #2
On 02/27/14 12:50, H.J. Lu wrote:
> On Thu, Feb 27, 2014 at 11:02 AM, Jeff Law <law@redhat.com> wrote:
>>
>> As discussed in 49847, a few years ago GCC was changed to add EH edges for
>> exceptions that might arise from a floating point comparison.  That change
>> made it possible for a cc0-setter and cc0-user to end up in different
>> blocks, separated by a NOTE.
>>
>> After discussing reverting the change, duplicating the cc0-setter and a
>> couple other ideas, eventually Richi and myself settled on relaxing the long
>> standing restrictions on the relative locations of the cc0 setter and cc0
>> user.
>>
>> We agreed to update the documentation and fault in fixes due to low priority
>> of cc0 targets (and doubly so since this really only affects cc0 targets
>> with used with non-call-exceptions).
>>
>> This patch updates the documentation and fixes the only known failure due to
>> this change in cse.c with a patch from Mikael.
>>
>> Tested with a cross compiler.  I'll probably fire off a m68k bootstrap for
>> good measure, but it'll be several days before that completes.
>>
>> Installed on the trunk.
>>
>> Jeff
>>
>> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
>> index 8a78716..a20cee3 100644
>> --- a/gcc/ChangeLog
>> +++ b/gcc/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2014-02-27  Mikael Pettersson  <mikpe@it.uu.se>
>> +           Jeff Law  <law@redhat.com>
>> +
>> +       PR rtl-optimization/49847
>> +       * cse.c (fold_rtx) Handle case where cc0 setter and cc0 user
>> +       are in different blocks.
>> +       * doc/tm.texi (Condition Code Status): Update documention for
>> +       relative locations of cc0-setter and cc0-user.
>> +
>
> This breaks bootstrap:
>
> You should edit ../../src-trunk/gcc/doc/tm.texi.in rather than
> ../../src-trunk/gcc/doc/tm.texi .
> make[6]: *** [s-tm-texi] Error 1
> make[6]: *** Waiting for unfinished jobs....
Nuts.  Sorry.  Thanks for fixing it.
jeff
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 8a78716..a20cee3 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@ 
+2014-02-27  Mikael Pettersson  <mikpe@it.uu.se>
+	    Jeff Law  <law@redhat.com>
+
+	PR rtl-optimization/49847
+	* cse.c (fold_rtx) Handle case where cc0 setter and cc0 user
+	are in different blocks.
+	* doc/tm.texi (Condition Code Status): Update documention for
+	relative locations of cc0-setter and cc0-user.
+
 2014-02-27  Vladimir Makarov  <vmakarov@redhat.com>
 
 	PR target/59222
diff --git a/gcc/cse.c b/gcc/cse.c
index cffa553..dba85f1 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -3199,9 +3199,27 @@  fold_rtx (rtx x, rtx insn)
 
 #ifdef HAVE_cc0
 	  case CC0:
-	    folded_arg = prev_insn_cc0;
-	    mode_arg = prev_insn_cc0_mode;
-	    const_arg = equiv_constant (folded_arg);
+	    /* The cc0-user and cc0-setter may be in different blocks if
+	       the cc0-setter potentially traps.  In that case PREV_INSN_CC0
+	       will have been cleared as we exited the block with the
+	       setter.
+
+	       While we could potentially track cc0 in this case, it just
+	       doesn't seem to be worth it given that cc0 targets are not
+	       terribly common or important these days and trapping math
+	       is rarely used.  The combination of those two conditions
+	       necessary to trip this situation is exceedingly rare in the
+	       real world.  */
+	    if (!prev_insn_cc0)
+	      {
+		const_arg = NULL_RTX;
+	      }
+	    else
+	      {
+		folded_arg = prev_insn_cc0;
+		mode_arg = prev_insn_cc0_mode;
+		const_arg = equiv_constant (folded_arg);
+	      }
 	    break;
 #endif
 
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index f204936..f7024a7 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -5900,8 +5900,13 @@  most instructions do not affect it.  The latter category includes
 most RISC machines.
 
 The implicit clobbering poses a strong restriction on the placement of
-the definition and use of the condition code, which need to be in adjacent
-insns for machines using @code{(cc0)}.  This can prevent important
+the definition and use of the condition code.  In the past the definition
+and use were always adjacent.  However, recent changes to support trapping
+arithmatic may result in the definition and user being in different blocks.
+Thus, there may be a @code{NOTE_INSN_BASIC_BLOCK} between them.  Additionally,
+the definition may be the source of exception handling edges.
+
+These restrictions can prevent important
 optimizations on some machines.  For example, on the IBM RS/6000, there
 is a delay for taken branches unless the condition code register is set
 three instructions earlier than the conditional branch.  The instruction
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 128a5f7..be4cb12 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-02-27  Mikael Pettersson  <mikpe@it.uu.se>
+            Jeff Law  <law@redhat.com>
+
+	 PR rtl-optimization/49847
+	 * g++.dg/pr49847.C: New test.
+
 2014-02-27  Marek Polacek  <polacek@redhat.com>
 
 	PR middle-end/59223
diff --git a/gcc/testsuite/g++.dg/pr49847.C b/gcc/testsuite/g++.dg/pr49847.C
new file mode 100644
index 0000000..b047713
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr49847.C
@@ -0,0 +1,7 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fnon-call-exceptions" } */
+int f (float g)
+{
+  try { return g >= 0; }
+  catch (...) {}
+}