diff mbox

cprop fix for PR78626

Message ID eefab903-071d-a8fe-887d-434f09c36abf@redhat.com
State New
Headers show

Commit Message

Bernd Schmidt Dec. 8, 2016, 12:21 p.m. UTC
This is another case where an optimization turns a trap_if 
unconditional. We have to defer changing the CFG, since the rest of 
cprop seems to blow up when we modify things while scanning.

Bootstrapped and tested on x86_64-linux, and reportedly fixes the 
problem on ppc, ok?


Bernd

Comments

Segher Boessenkool Dec. 10, 2016, 7:58 p.m. UTC | #1
On Thu, Dec 08, 2016 at 01:21:04PM +0100, Bernd Schmidt wrote:
> This is another case where an optimization turns a trap_if 
> unconditional. We have to defer changing the CFG, since the rest of 
> cprop seems to blow up when we modify things while scanning.
> 
> Bootstrapped and tested on x86_64-linux, and reportedly fixes the 
> problem on ppc, ok?

This fixes PR78626, but not yet PR78727.

> @@ -1825,11 +1828,26 @@ one_cprop_pass (void)
>  		       insn into a NOTE, or deleted the insn.  */
>  		if (! NOTE_P (insn) && ! insn->deleted ())
>  		  mark_oprs_set (insn);
> +
> +		if (first_uncond_trap == NULL
> +		    && GET_CODE (PATTERN (insn)) == TRAP_IF
> +		    && XEXP (PATTERN (insn), 0) == const1_rtx)
> +		  first_uncond_trap = insn;
>  	      }
> +	  if (first_uncond_trap != NULL && first_uncond_trap != BB_END (bb))
> +	    uncond_traps.safe_push (first_uncond_trap);
>  	}

The problem for PR78727 is that we also need to do this for insns that
already are the last insn in the block:

> +      while (!uncond_traps.is_empty ())
> +	{
> +	  rtx_insn *insn = uncond_traps.pop ();
> +	  basic_block to_split = BLOCK_FOR_INSN (insn);
> +	  remove_edge (split_block (to_split, insn));
> +	  emit_barrier_after_bb (to_split);
> +	}

We need that barrier, and we also need the successor edges removed
(which split_block+remove_edge does).

(PR78727 works fine with just that BB_END test deleted).


Segher
Bernd Schmidt Dec. 12, 2016, 2:21 p.m. UTC | #2
On 12/10/2016 08:58 PM, Segher Boessenkool wrote:
> On Thu, Dec 08, 2016 at 01:21:04PM +0100, Bernd Schmidt wrote:
>> This is another case where an optimization turns a trap_if
>> unconditional. We have to defer changing the CFG, since the rest of
>> cprop seems to blow up when we modify things while scanning.

> The problem for PR78727 is that we also need to do this for insns that
> already are the last insn in the block:
>
>> +      while (!uncond_traps.is_empty ())
>> +	{
>> +	  rtx_insn *insn = uncond_traps.pop ();
>> +	  basic_block to_split = BLOCK_FOR_INSN (insn);
>> +	  remove_edge (split_block (to_split, insn));
>> +	  emit_barrier_after_bb (to_split);
>> +	}
>
> We need that barrier, and we also need the successor edges removed
> (which split_block+remove_edge does).
>
> (PR78727 works fine with just that BB_END test deleted).

Ah, ok. In that case I'll probably also add a test to make sure this is 
only done for insns that weren't an unconditional trap before. Retesting...


Bernd
diff mbox

Patch

	PR rtl-optimization/78626
	* cprop.c (one_cprop_pass): Collect unconditional traps in the middle
	of a block, and split such blocks after everything else is finished.

	PR rtl-optimization/78626
	* gcc.dg/torture/pr78626.c: New test.

Index: gcc/cprop.c
===================================================================
--- gcc/cprop.c	(revision 243310)
+++ gcc/cprop.c	(working copy)
@@ -1794,7 +1794,7 @@  one_cprop_pass (void)
   if (set_hash_table.n_elems > 0)
     {
       basic_block bb;
-      rtx_insn *insn;
+      auto_vec<rtx_insn *> uncond_traps;
 
       alloc_cprop_mem (last_basic_block_for_fn (cfun),
 		       set_hash_table.n_elems);
@@ -1810,6 +1810,9 @@  one_cprop_pass (void)
 		      EXIT_BLOCK_PTR_FOR_FN (cfun),
 		      next_bb)
 	{
+	  rtx_insn *first_uncond_trap = NULL;
+	  rtx_insn *insn;
+
 	  /* Reset tables used to keep track of what's still valid [since
 	     the start of the block].  */
 	  reset_opr_set_tables ();
@@ -1825,11 +1828,26 @@  one_cprop_pass (void)
 		       insn into a NOTE, or deleted the insn.  */
 		if (! NOTE_P (insn) && ! insn->deleted ())
 		  mark_oprs_set (insn);
+
+		if (first_uncond_trap == NULL
+		    && GET_CODE (PATTERN (insn)) == TRAP_IF
+		    && XEXP (PATTERN (insn), 0) == const1_rtx)
+		  first_uncond_trap = insn;
 	      }
+	  if (first_uncond_trap != NULL && first_uncond_trap != BB_END (bb))
+	    uncond_traps.safe_push (first_uncond_trap);
 	}
 
       changed |= bypass_conditional_jumps ();
 
+      while (!uncond_traps.is_empty ())
+	{
+	  rtx_insn *insn = uncond_traps.pop ();
+	  basic_block to_split = BLOCK_FOR_INSN (insn);
+	  remove_edge (split_block (to_split, insn));
+	  emit_barrier_after_bb (to_split);
+	}
+
       FREE_REG_SET (reg_set_bitmap);
       free_cprop_mem ();
     }
Index: gcc/testsuite/gcc.dg/torture/pr78626.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr78626.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr78626.c	(working copy)
@@ -0,0 +1,27 @@ 
+/* { dg-do compile } */
+
+int qs;
+
+void
+ms (int g1)
+{
+  int cy;
+  int *fr = &cy;
+
+  for (;;)
+    {
+      *fr = 1;
+      fr = &g1;
+
+      while (qs != 0)
+        {
+          if (qs | cy)
+            qs = g1 / 0; /* { dg-warning "division" } */
+          ++qs;
+        }
+
+      cy = 1;
+      while (cy != 0)
+        cy = 2;
+    }
+}