[PATCH] Disable remove_exit_barrier OpenMP optimization when it might break tasking (PR other/39591)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

As shown on the attached testcases, the remove_exit_barrier optimization
can in some cases tasks using shared vars, because without this optimization
there is a barrier on which the queued up tasks are run, but without it
some parallel region's variables can be already out of scope when the tasks
are dispatched.  Unfortunately it isn't just when explicit task region is inside
of the parallel region (which could be easily checked at compile time), but
the task can be created by some other function (see *-2.c testcase), as long
as the task could get to an address of a variable that goes out of scope in
between the two barriers.

The following patch disables the optimization if there are any addressable
variables that might go out of scope in between the two barriers, I'm afraid
doing inter-procedure analysis of what functions could ever create explicit
tasks would be overkill.

Bootstrapped/regtested on x86_64-linux, will apply later tonight, unless
somebody comes up with some bright idea how to find out other safe cases
when it is possible to optimize.

2009-03-31  Jakub Jelinek  <jakub@redhat.com>

	PR other/39591
	* omp-low.c (remove_exit_barrier): Don't optimize if there are any
	addressable variables in the parallel that could go out of scope while
	running queued tasks.

	* testsuite/libgomp.c/pr39591-1.c: New test.
	* testsuite/libgomp.c/pr39591-2.c: New test.
	* testsuite/libgomp.c/pr39591-3.c: New test.

--- gcc/omp-low.c.jj	2009-03-30 12:46:01.000000000 +0200
+++ gcc/omp-low.c	2009-03-31 11:44:55.000000000 +0200
@@ -3,7 +3,7 @@
    marshalling to implement data sharing and copying clauses.
    Contributed by Diego Novillo <dnovillo@redhat.com>
 
-   Copyright (C) 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+   Copyright (C) 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
 
 This file is part of GCC.
 
@@ -3123,6 +3123,7 @@ remove_exit_barrier (struct omp_region *
   edge_iterator ei;
   edge e;
   gimple stmt;
+  int any_addressable_vars = -1;
 
   exit_bb = region->exit;
 
@@ -3148,8 +3149,52 @@ remove_exit_barrier (struct omp_region *
       if (gsi_end_p (gsi))
 	continue;
       stmt = gsi_stmt (gsi);
-      if (gimple_code (stmt) == GIMPLE_OMP_RETURN)
-	gimple_omp_return_set_nowait (stmt);
+      if (gimple_code (stmt) == GIMPLE_OMP_RETURN
+	  && !gimple_omp_return_nowait_p (stmt))
+	{
+	  /* OpenMP 3.0 tasks unfortunately prevent this optimization
+	     in many cases.  If there could be tasks queued, the barrier
+	     might be needed to let the tasks run before some local
+	     variable of the parallel that the task uses as shared
+	     runs out of scope.  The task can be spawned either
+	     from within current function (this would be easy to check)
+	     or from some function it calls and gets passed an address
+	     of such a variable.  */
+	  if (any_addressable_vars < 0)
+	    {
+	      gimple parallel_stmt = last_stmt (region->entry);
+	      tree child_fun = gimple_omp_parallel_child_fn (parallel_stmt);
+	      tree local_decls = DECL_STRUCT_FUNCTION (child_fun)->local_decls;
+	      tree block;
+
+	      any_addressable_vars = 0;
+	      for (; local_decls; local_decls = TREE_CHAIN (local_decls))
+		if (TREE_ADDRESSABLE (TREE_VALUE (local_decls)))
+		  {
+		    any_addressable_vars = 1;
+		    break;
+		  }
+	      for (block = gimple_block (stmt);
+		   !any_addressable_vars
+		   && block
+		   && TREE_CODE (block) == BLOCK;
+		   block = BLOCK_SUPERCONTEXT (block))
+		{
+		  for (local_decls = BLOCK_VARS (block);
+		       local_decls;
+		       local_decls = TREE_CHAIN (local_decls))
+		    if (TREE_ADDRESSABLE (local_decls))
+		      {
+			any_addressable_vars = 1;
+			break;
+		      }
+		  if (block == gimple_block (parallel_stmt))
+		    break;
+		}
+	    }
+	  if (!any_addressable_vars)
+	    gimple_omp_return_set_nowait (stmt);
+	}
     }
 }
 
--- libgomp/testsuite/libgomp.c/pr39591-1.c.jj	2009-03-31 11:48:44.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr39591-1.c	2009-03-31 10:14:11.000000000 +0200
@@ -0,0 +1,33 @@
+/* PR other/39591 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+int err;
+
+int
+main (void)
+{
+#pragma omp parallel
+  {
+    int array[40];
+    int i;
+    for (i = 0; i < sizeof array / sizeof array[0]; i++)
+      array[i] = 0x55555555;
+
+#pragma omp for schedule(dynamic)
+    for (i = 0; i < 50; i++)
+#pragma omp task shared(array)
+      {
+	int j;
+	for (j = 0; j < sizeof array / sizeof array[0]; j++)
+	  if (array[j] != 0x55555555)
+#pragma omp atomic
+	    err++;
+      }
+  }
+  if (err)
+    abort ();
+  return 0;
+}
--- libgomp/testsuite/libgomp.c/pr39591-2.c.jj	2009-03-31 11:48:47.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr39591-2.c	2009-03-31 12:12:27.000000000 +0200
@@ -0,0 +1,39 @@
+/* PR other/39591 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+int err;
+
+void __attribute__((noinline))
+foo (int *array)
+{
+#pragma omp task
+  {
+    int j;
+    for (j = 0; j < sizeof array / sizeof array[0]; j++)
+      if (array[j] != 0x55555555)
+#pragma omp atomic
+	err++;
+  }
+}
+
+int
+main (void)
+{
+#pragma omp parallel
+  {
+    int array[40];
+    int i;
+    for (i = 0; i < sizeof array / sizeof array[0]; i++)
+      array[i] = 0x55555555;
+
+#pragma omp for schedule (dynamic)
+    for (i = 0; i < 50; i++)
+      foo (array);
+  }
+  if (err)
+    abort ();
+  return 0;
+}
--- libgomp/testsuite/libgomp.c/pr39591-3.c.jj	2009-03-31 11:48:49.000000000 +0200
+++ libgomp/testsuite/libgomp.c/pr39591-3.c	2009-03-31 12:12:34.000000000 +0200
@@ -0,0 +1,40 @@
+/* PR other/39591 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+extern void abort (void);
+
+int err, a[40];
+
+void __attribute__((noinline))
+foo (int *array)
+{
+#pragma omp task
+  {
+    int j;
+    for (j = 0; j < sizeof array / sizeof array[0]; j++)
+      if (array[j] != 0x55555555)
+#pragma omp atomic
+	err++;
+  }
+}
+
+int
+main (void)
+{
+  int k;
+  for (k = 0; k < sizeof a / sizeof a[0]; k++)
+    a[k] = 0x55555555;
+
+#pragma omp parallel
+  {
+    int i;
+
+#pragma omp for schedule (dynamic)
+    for (i = 0; i < 50; i++)
+      foo (a);
+  }
+  if (err)
+    abort ();
+  return 0;
+}

	Jakub