Don't cross-jump in between frame related and non-frame related insns (PR target/80102)

Submitted by Jakub Jelinek on March 20, 2017, 9:15 p.m.

Details

Message ID 20170320211520.GO11094@tucnak
State New
Headers show

Commit Message

Jakub Jelinek March 20, 2017, 9:15 p.m.
Hi!

On ppc64le we ICE on the following testcase, because during jump2 we decide
to cross-jump
(insn/f 62 61 63 5 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_BLOCK) "pr80102.C":9 -1
     (expr_list:REG_CFA_RESTORE (reg:DI 30 30)
        (nil)))
(jump_insn 63 62 90 5 (simple_return) "pr80102.C":9 -1
     (nil)
with
(insn 73 72 84 8 (unspec_volatile [
            (const_int 0 [0])
        ] UNSPECV_BLOCK) "pr80102.C":9 504 {blockage}
     (nil))
(jump_insn 84 73 85 8 (simple_return) "pr80102.C":9 -1
     (nil)
Note that one of the blockage insns holds CFI instructions on them, while
the other doesn't (admittedly it is very weird to attach this to an empty
insn, but that is what the rs6000 backend does).

The following patch fixes that by avoiding to cross-jump them, because
that will often read to unwind info inconsistencies.
Not really sure what we should do if both i1 and i2 are frame related, shall
we check for each of the CFA reg notes if they are available and equal?
Or punt if either of the insns is frame related?

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-03-20  Jakub Jelinek  <jakub@redhat.com>

	PR target/80102
	* cfgcleanup.c (old_insns_match_p): Don't cross-jump in between /f
	and non-/f instructions.

	* g++.dg/opt/pr80102.C: New test.


	Jakub

Comments

Richard Henderson March 20, 2017, 10:38 p.m.
On 03/21/2017 07:15 AM, Jakub Jelinek wrote:
> Not really sure what we should do if both i1 and i2 are frame related, shall
> we check for each of the CFA reg notes if they are available and equal?
> Or punt if either of the insns is frame related?

I would punt if either is frame related.

As an aside, if the length of "blockage" is corrected to 0, does cross-jumping 
skip this case?  Because replacing a simple_return with a direct branch to a 
simple_return is not a win.  But of course at the moment cross-jumping thinks 
it is eliminating the second blockage as well...


r~

Patch hide | download patch | download mbox

--- gcc/cfgcleanup.c.jj	2017-01-09 22:46:03.000000000 +0100
+++ gcc/cfgcleanup.c	2017-03-20 13:55:58.823983848 +0100
@@ -1149,6 +1149,11 @@  old_insns_match_p (int mode ATTRIBUTE_UN
   else if (p1 || p2)
     return dir_none;
 
+  /* Do not allow cross-jumping between frame related insns and other
+     insns.  */
+  if (RTX_FRAME_RELATED_P (i1) != RTX_FRAME_RELATED_P (i2))
+    return dir_none;
+
   p1 = PATTERN (i1);
   p2 = PATTERN (i2);
 
--- gcc/testsuite/g++.dg/opt/pr80102.C.jj	2017-03-20 14:34:01.223434828 +0100
+++ gcc/testsuite/g++.dg/opt/pr80102.C	2017-03-20 14:33:36.000000000 +0100
@@ -0,0 +1,14 @@ 
+// PR target/80102
+// { dg-do compile }
+// { dg-options "-fnon-call-exceptions -Os" }
+// { dg-additional-options "-mminimal-toc" { target { powerpc*-*-* && lp64 } } }
+
+struct B { float a; B (float c) { for (int g; g < c;) ++a; } };
+struct D { D (B); };
+
+int
+main ()
+{
+  B (1.0);
+  D e (0.0), f (1.0);
+}