Fix RTL DCE with separate shrink wrapped epilogues (PR rtl-optimization/83985)

Message ID 20180125222033.GT2063@tucnak
State New
Headers show
Series
  • Fix RTL DCE with separate shrink wrapped epilogues (PR rtl-optimization/83985)
Related show

Commit Message

Jakub Jelinek Jan. 25, 2018, 10:20 p.m.
Hi!

The r241060 change added the second hunk in this patch which the patch is
reverting.  The problem is that not deleting some unmarked insns in
delete_unmarked_insns is done in a wrong place, it causes indeed not to
delete the instruction we don't want to DCE, but the instructions that
are needed by the instructions (in this case a memory load whose result
the REG_CFA_RESTORE instruction uses) are not marked either and those are
deleted.

The following patch fixes it by making such instructions non-deletable,
which means they are marked and the DCE algorithm then marks the
instructions they need too.

Bootstrapped/regtested on {x86_64,i686,powerpc64{,le}}-linux, ok for trunk?

2018-01-25  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/83985
	* dce.c (deletable_insn_p): Return false for separate shrink wrapping
	REG_CFA_RESTORE insns.
	(delete_unmarked_insns): Don't ignore separate shrink wrapping
	REG_CFA_RESTORE insns here.

	* gcc.dg/pr83985.c: New test.


	Jakub

Comments

Segher Boessenkool Jan. 25, 2018, 11:48 p.m. | #1
On Thu, Jan 25, 2018 at 11:20:33PM +0100, Jakub Jelinek wrote:
> Hi!

> 

> The r241060 change added the second hunk in this patch which the patch is

> reverting.  The problem is that not deleting some unmarked insns in

> delete_unmarked_insns is done in a wrong place, it causes indeed not to

> delete the instruction we don't want to DCE, but the instructions that

> are needed by the instructions (in this case a memory load whose result

> the REG_CFA_RESTORE instruction uses) are not marked either and those are

> deleted.

> 

> The following patch fixes it by making such instructions non-deletable,

> which means they are marked and the DCE algorithm then marks the

> instructions they need too.


Looks good to me!  Thanks.  And sorry for causing the bug in the first
place :-/


Segher
Richard Biener Jan. 26, 2018, 11:16 a.m. | #2
On Thu, 25 Jan 2018, Segher Boessenkool wrote:

> On Thu, Jan 25, 2018 at 11:20:33PM +0100, Jakub Jelinek wrote:

> > Hi!

> > 

> > The r241060 change added the second hunk in this patch which the patch is

> > reverting.  The problem is that not deleting some unmarked insns in

> > delete_unmarked_insns is done in a wrong place, it causes indeed not to

> > delete the instruction we don't want to DCE, but the instructions that

> > are needed by the instructions (in this case a memory load whose result

> > the REG_CFA_RESTORE instruction uses) are not marked either and those are

> > deleted.

> > 

> > The following patch fixes it by making such instructions non-deletable,

> > which means they are marked and the DCE algorithm then marks the

> > instructions they need too.

> 

> Looks good to me!  Thanks.  And sorry for causing the bug in the first

> place :-/


Ok.

Richard.

Patch

--- gcc/dce.c.jj	2018-01-04 00:43:17.995703342 +0100
+++ gcc/dce.c	2018-01-25 17:55:49.750007894 +0100
@@ -131,6 +131,12 @@  deletable_insn_p (rtx_insn *insn, bool f
 	     && REGNO (pic_offset_table_rtx) >= FIRST_PSEUDO_REGISTER)
       return false;
 
+  /* Callee-save restores are needed.  */
+  if (RTX_FRAME_RELATED_P (insn)
+      && crtl->shrink_wrapped_separate
+      && find_reg_note (insn, REG_CFA_RESTORE, NULL))
+    return false;
+
   body = PATTERN (insn);
   switch (GET_CODE (body))
     {
@@ -592,15 +598,6 @@  delete_unmarked_insns (void)
 	  if (!dbg_cnt (dce))
 	    continue;
 
-	  if (crtl->shrink_wrapped_separate
-	      && find_reg_note (insn, REG_CFA_RESTORE, NULL))
-	    {
-	      if (dump_file)
-		fprintf (dump_file, "DCE: NOT deleting insn %d, it's a "
-				    "callee-save restore\n", INSN_UID (insn));
-	      continue;
-	    }
-
 	  if (dump_file)
 	    fprintf (dump_file, "DCE: Deleting insn %d\n", INSN_UID (insn));
 
--- gcc/testsuite/gcc.dg/pr83985.c.jj	2018-01-25 18:05:23.366121812 +0100
+++ gcc/testsuite/gcc.dg/pr83985.c	2018-01-25 18:05:03.513117871 +0100
@@ -0,0 +1,25 @@ 
+/* PR rtl-optimization/83985 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-additional-options "-mcpu=e300c3 -mtune=e300c3" { target { powerpc*-*-* && ilp32 } } } */
+
+long long int v;
+
+void
+foo (int x)
+{
+  if (x == 0)
+    return;
+
+  while (v < 2)
+    {
+      signed char *a;
+      v /= x;
+      a = v == 0 ? (signed char *) &x : (signed char *) &v;
+      ++*a;
+      ++v;
+    }
+
+  while (1)
+    ;
+}