diff mbox

Handle redirection blocks with clobbers

Message ID 553533DB.3000903@redhat.com
State New
Headers show

Commit Message

Jeff Law April 20, 2015, 5:14 p.m. UTC 
PR 65658 shows a case where we fail to thread jumps in a block that is 
trivially threadable and would generate no code if threaded.  That in 
turn results in inefficient code and a false positive from -Wuninitialized.

The problem is the problem block has a clobber statement and 
redirection_block_p thus rejects the block as a redirection block.  That 
in turn causes the recorded jump threads to be pruned.

Fixed by handling clobber statements in redirection-block_p.

Bootstrapped and regression tested on x86-linux-gnu.  Installed on the 
trunk.

Jeff

Comments

Jakub Jelinek April 20, 2015, 5:19 p.m. UTC | #1 
On Mon, Apr 20, 2015 at 11:14:03AM -0600, Jeff Law wrote:
>    while (!gsi_end_p (gsi)
>  	 && (gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL
>  	     || is_gimple_debug (gsi_stmt (gsi))
> -	     || gimple_nop_p (gsi_stmt (gsi))))
> +	     || gimple_nop_p (gsi_stmt (gsi))
> +	     || (gimple_code (gsi_stmt (gsi)) == GIMPLE_ASSIGN

Why this line?  There is no need to test for GIMPLE_ASSIGN
(and canonical test for that would be is_gimple_assign (gsi_stmt (gsi))
anyway),

> +		 && gimple_clobber_p (gsi_stmt (gsi)))))

gimple_clobber_p checks for that too.

	Jakub
Jeff Law April 20, 2015, 7:07 p.m. UTC | #2 
On 04/20/2015 11:19 AM, Jakub Jelinek wrote:
> On Mon, Apr 20, 2015 at 11:14:03AM -0600, Jeff Law wrote:
>>     while (!gsi_end_p (gsi)
>>   	 && (gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL
>>   	     || is_gimple_debug (gsi_stmt (gsi))
>> -	     || gimple_nop_p (gsi_stmt (gsi))))
>> +	     || gimple_nop_p (gsi_stmt (gsi))
>> +	     || (gimple_code (gsi_stmt (gsi)) == GIMPLE_ASSIGN
>
> Why this line?  There is no need to test for GIMPLE_ASSIGN
> (and canonical test for that would be is_gimple_assign (gsi_stmt (gsi))
> anyway),
>
>> +		 && gimple_clobber_p (gsi_stmt (gsi)))))
>
> gimple_clobber_p checks for that too.
Build spinning with the redundant check removed...

jeff
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a34e846..9d8c2bc 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@ 
+2015-04-20  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/65658
+	* tree-ssa-threadupdate.c (redirection_block_p): Ignore clobber
+	statements too.
+
 2015-04-20  Alan Lawrence  <alan.lawrence@arm.com>
 
 	* config/aarch64/aarch64.c (aarch64_simd_emit_pair_result_insn): Delete.
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 248ffc8..e768f57 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2015-04-20  Jeff Law  <law@redhat.com>
+
+	PR tree-optimization/65658
+	* gcc.dg/pr65658.c: New test.
+
 2015-04-20  Alan Lawrence  <alan.lawrence@arm.com>
 
 	PR target/64134
diff --git a/gcc/testsuite/gcc.dg/pr65658.c b/gcc/testsuite/gcc.dg/pr65658.c
new file mode 100644
index 0000000..cce0f2a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr65658.c
@@ -0,0 +1,111 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wuninitialized -O2 -Wno-implicit" } */
+
+extern int optind;
+struct undefinfo
+{
+  unsigned long l1;
+  unsigned long l2;
+};
+struct undeffoo
+{
+  char a[64];
+  long b[4];
+  int c[33];
+};
+struct problem
+{
+  unsigned long l1;
+  unsigned long l2;
+  unsigned long l3;
+  unsigned long l4;
+};
+static unsigned int undef1, undef2, undef3, undef4, undef5, undef6;
+static void *undefvp1;
+extern struct undefinfo undefinfo;
+static int
+undefinit1 (void)
+{
+  struct undeffoo foo;
+  int i;
+  for (i = 0; i < 2000; i++)
+    {
+      undef6++;
+      external_function5 (((void *) 0), 0, (void *) &foo);
+    }
+}
+
+static int
+undefinit2 (void *problemp, unsigned long problem)
+{
+  int ret, u;
+  if (undefinit1 ())
+    return 1;
+  if (fn10 ())
+    return 1;
+  for (u = 0; u < undef6; u++)
+    {
+      ret = external_function1 (3 + u * 10, 10);
+      if (ret)
+	return ret;
+      external_function6 (0, 0, 0, problemp + problem);
+      return 1;
+    }
+}
+
+static int
+fn6 (struct undefinfo *uip, struct problem *problem)
+{
+  unsigned long amt;
+  if (external_function3 (((void *) 0), ((void *) 0), &amt, 0, 0))
+    return 1;
+  problem->l1 = (unsigned long) undefvp1;
+  problem->l4 = uip->l1;
+  problem->l3 = uip->l2;
+  return 0;
+}
+
+static int
+setup (void)
+{
+  struct problem problem;
+  if (fn6 (&undefinfo, &problem))
+    return 1;
+  if (fn2 ())
+    return 1;
+  if (fn4 (101))
+    return 1;
+  if (undefinit2 ((void *) problem.l1, problem.l3 * 4))  /* { dg-bogus "problem.l3" "uninitialized variable warning" } */ 
+    return 1;
+}
+
+int
+main (int argc, char **argv)
+{
+  int optc;
+  if (external_function (1))
+    return 1;
+  if (external_function (1))
+    return 1;
+  if (external_function (1))
+    return 1;
+  while ((optc =
+	  getopt_long (argc, argv, ((void *) 0), ((void *) 0),
+		       ((void *) 0))) != -1)
+    {
+      switch (optc)
+	{
+	case 0:
+	  break;
+	case 'F':
+	  external_function (1);
+	default:
+	  return 1;
+	}
+    }
+  if ((optind != 99))
+    {
+      return 1;
+    }
+  setup ();
+}
diff --git a/gcc/tree-ssa-threadupdate.c b/gcc/tree-ssa-threadupdate.c
index 709b16e..9f263bd 100644
--- a/gcc/tree-ssa-threadupdate.c
+++ b/gcc/tree-ssa-threadupdate.c
@@ -1449,7 +1449,9 @@  redirection_block_p (basic_block bb)
   while (!gsi_end_p (gsi)
 	 && (gimple_code (gsi_stmt (gsi)) == GIMPLE_LABEL
 	     || is_gimple_debug (gsi_stmt (gsi))
-	     || gimple_nop_p (gsi_stmt (gsi))))
+	     || gimple_nop_p (gsi_stmt (gsi))
+	     || (gimple_code (gsi_stmt (gsi)) == GIMPLE_ASSIGN
+		 && gimple_clobber_p (gsi_stmt (gsi)))))
     gsi_next (&gsi);
 
   /* Check if this is an empty block.  */