This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[committed] Fix #pragma omp task expansion with noreturn task bodies (PR middle-end/66133)


Hi!

During taskloop development, I have noticed a bug in the GIMPLE_OMP_TASK
ompexp - unlike GIMPLE_OMP_PARALLEL, where if the parallel body is noreturn,
the GOMP_parallel call never returns either, for tasks it really depends
on whether the task is included or not.  If the library (the usual case)
schedules it to run in some other thread, or the same one when it starts
waiting on a barrier, then it will return immediately.
Without this patch we weren't representing this possible control flow
through a cfg edge (from cfg pass till ompexp), which meant that we
in that case assumed the GOMP_task call will not return.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk.  Will backport after a few days to 5 branch, perhaps
older branches too.

2015-05-13  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/66133
	* omp-low.c (expand_omp_taskreg): For GIMPLE_OMP_TASK expansion,
	make sure it is never noreturn, even when the task body does not
	return.
	(lower_omp_taskreg): For GIMPLE_OMP_TASK, emit GIMPLE_OMP_CONTINUE
	right before GIMPLE_OMP_RETURN.
	(make_gimple_omp_edges): Accept GIMPLE_OMP_CONTINUE as ->cont
	for GIMPLE_OMP_TASK.  For GIMPLE_OMP_RETURN corresponding to
	GIMPLE_OMP_TASK add an EDGE_ABNORMAL edge from entry to exit.

	* testsuite/libgomp.c/pr66133.c: New test.

--- gcc/omp-low.c.jj	2015-04-21 18:23:24.000000000 +0200
+++ gcc/omp-low.c	2015-05-13 15:41:03.648446718 +0200
@@ -5377,7 +5377,10 @@ expand_omp_taskreg (struct omp_region *r
   child_cfun = DECL_STRUCT_FUNCTION (child_fn);
 
   entry_bb = region->entry;
-  exit_bb = region->exit;
+  if (gimple_code (entry_stmt) == GIMPLE_OMP_TASK)
+    exit_bb = region->cont;
+  else
+    exit_bb = region->exit;
 
   bool is_cilk_for
     = (flag_cilkplus
@@ -5436,7 +5439,9 @@ expand_omp_taskreg (struct omp_region *r
 	 variable.  In which case, we need to keep the assignment.  */
       if (gimple_omp_taskreg_data_arg (entry_stmt))
 	{
-	  basic_block entry_succ_bb = single_succ (entry_bb);
+	  basic_block entry_succ_bb
+	    = single_succ_p (entry_bb) ? single_succ (entry_bb)
+				       : FALLTHRU_EDGE (entry_bb)->dest;
 	  tree arg, narg;
 	  gimple parcopy_stmt = NULL;
 
@@ -5524,14 +5529,28 @@ expand_omp_taskreg (struct omp_region *r
       e = split_block (entry_bb, stmt);
       gsi_remove (&gsi, true);
       entry_bb = e->dest;
-      single_succ_edge (entry_bb)->flags = EDGE_FALLTHRU;
+      edge e2 = NULL;
+      if (gimple_code (entry_stmt) == GIMPLE_OMP_PARALLEL)
+	single_succ_edge (entry_bb)->flags = EDGE_FALLTHRU;
+      else
+	{
+	  e2 = make_edge (e->src, BRANCH_EDGE (entry_bb)->dest, EDGE_ABNORMAL);
+	  gcc_assert (e2->dest == region->exit);
+	  remove_edge (BRANCH_EDGE (entry_bb));
+	  set_immediate_dominator (CDI_DOMINATORS, e2->dest, e->src);
+	  gsi = gsi_last_bb (region->exit);
+	  gcc_assert (!gsi_end_p (gsi)
+		      && gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_RETURN);
+	  gsi_remove (&gsi, true);
+	}
 
-      /* Convert GIMPLE_OMP_RETURN into a RETURN_EXPR.  */
+      /* Convert GIMPLE_OMP_{RETURN,CONTINUE} into a RETURN_EXPR.  */
       if (exit_bb)
 	{
 	  gsi = gsi_last_bb (exit_bb);
 	  gcc_assert (!gsi_end_p (gsi)
-		      && gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_RETURN);
+		      && (gimple_code (gsi_stmt (gsi))
+			  == (e2 ? GIMPLE_OMP_CONTINUE : GIMPLE_OMP_RETURN)));
 	  stmt = gimple_build_return (NULL);
 	  gsi_insert_after (&gsi, stmt, GSI_SAME_STMT);
 	  gsi_remove (&gsi, true);
@@ -5552,6 +5571,14 @@ expand_omp_taskreg (struct omp_region *r
       new_bb = move_sese_region_to_fn (child_cfun, entry_bb, exit_bb, block);
       if (exit_bb)
 	single_succ_edge (new_bb)->flags = EDGE_FALLTHRU;
+      if (e2)
+	{
+	  basic_block dest_bb = e2->dest;
+	  if (!exit_bb)
+	    make_edge (new_bb, dest_bb, EDGE_FALLTHRU);
+	  remove_edge (e2);
+	  set_immediate_dominator (CDI_DOMINATORS, dest_bb, new_bb);
+	}
       /* When the OMP expansion process cannot guarantee an up-to-date
          loop tree arrange for the child function to fixup loops.  */
       if (loops_state_satisfies_p (LOOPS_NEED_FIXUP))
@@ -11158,6 +11185,10 @@ lower_omp_taskreg (gimple_stmt_iterator
     gimple_seq_add_stmt (&new_body, gimple_build_label (ctx->cancel_label));
   gimple_seq_add_seq (&new_body, par_olist);
   new_body = maybe_catch_exception (new_body);
+  if (gimple_code (stmt) == GIMPLE_OMP_TASK)
+    gimple_seq_add_stmt (&new_body,
+			 gimple_build_omp_continue (integer_zero_node,
+						    integer_zero_node));
   gimple_seq_add_stmt (&new_body, gimple_build_omp_return (false));
   gimple_omp_set_body (stmt, new_body);
 
@@ -12272,6 +12303,10 @@ make_gimple_omp_edges (basic_block bb, s
 	 somewhere other than the next block.  This will be
 	 created later.  */
       cur_region->exit = bb;
+      if (cur_region->type == GIMPLE_OMP_TASK)
+	/* Add an edge corresponding to not scheduling the task
+	   immediately.  */
+	make_edge (cur_region->entry, bb, EDGE_ABNORMAL);
       fallthru = cur_region->type != GIMPLE_OMP_SECTION;
       cur_region = cur_region->outer;
       break;
@@ -12320,6 +12355,10 @@ make_gimple_omp_edges (basic_block bb, s
 	  }
 	  break;
 
+	case GIMPLE_OMP_TASK:
+	  fallthru = true;
+	  break;
+
 	default:
 	  gcc_unreachable ();
 	}
--- libgomp/testsuite/libgomp.c/pr66133.c.jj	2015-05-13 13:44:05.592658643 +0200
+++ libgomp/testsuite/libgomp.c/pr66133.c	2015-05-13 13:45:13.974586586 +0200
@@ -0,0 +1,35 @@
+/* PR middle-end/66133 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fopenmp" } */
+
+#include <stdlib.h>
+#include <unistd.h>
+
+volatile int x;
+
+__attribute__((noinline)) void
+foo (void)
+{
+  if (x == 0)
+    {
+      #pragma omp task
+	{
+	  usleep (2000);
+	  exit (0);
+	}
+    }
+  else
+    abort ();
+}
+
+int
+main ()
+{
+  #pragma omp parallel num_threads (2)
+    {
+      #pragma omp barrier
+      #pragma omp single
+	foo ();
+    }
+  exit (0);
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]