Patchwork [committed] Fix #pragma omp sections with all sections non-returning (PR middle-end/54017)

login
register
mail settings
Submitter Jakub Jelinek
Date July 19, 2012, 2:20 p.m.
Message ID <20120719142032.GB4807@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/171952/
State New
Headers show

Comments

Jakub Jelinek - July 19, 2012, 2:20 p.m.
Hi!

If all #pragma omp section blocks of #pragma omp sections never exit,
and a program invoking that is executed on more threads than sections,
we SIGILL due to __builtin_trap, as opposed to waiting forever for the
looping sections.  Fixed thusly (primarily because we can remove code,
the testcase is very questionable), bootstrapped/regtested on x86_64-linux
and i686-linux, committed to trunk and 4.7.

2012-07-19  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/54017
	* tree-cfgcleanup.c (cleanup_omp_return): Remove.
	(cleanup_tree_cfg_bb): Don't call it.
	* omp-low.c (expand_omp_sections): Fix up the !exit_reachable case
	handling.

	* c-c++-common/gomp/pr54017.c: New test.


	Jakub

Patch

--- gcc/tree-cfgcleanup.c.jj	2012-07-16 14:38:34.000000000 +0200
+++ gcc/tree-cfgcleanup.c	2012-07-19 11:33:31.919718954 +0200
@@ -606,48 +606,13 @@  split_bbs_on_noreturn_calls (void)
   return changed;
 }
 
-/* If GIMPLE_OMP_RETURN in basic block BB is unreachable, remove it.  */
-
-static bool
-cleanup_omp_return (basic_block bb)
-{
-  gimple stmt = last_stmt (bb);
-  basic_block control_bb;
-
-  if (stmt == NULL
-      || gimple_code (stmt) != GIMPLE_OMP_RETURN
-      || !single_pred_p (bb))
-    return false;
-
-  control_bb = single_pred (bb);
-  stmt = last_stmt (control_bb);
-
-  if (stmt == NULL || gimple_code (stmt) != GIMPLE_OMP_SECTIONS_SWITCH)
-    return false;
-
-  /* The block with the control statement normally has two entry edges -- one
-     from entry, one from continue.  If continue is removed, return is
-     unreachable, so we remove it here as well.  */
-  if (EDGE_COUNT (control_bb->preds) == 2)
-    return false;
-
-  gcc_assert (EDGE_COUNT (control_bb->preds) == 1);
-  remove_edge_and_dominated_blocks (single_pred_edge (bb));
-  return true;
-}
-
 /* Tries to cleanup cfg in basic block BB.  Returns true if anything
    changes.  */
 
 static bool
 cleanup_tree_cfg_bb (basic_block bb)
 {
-  bool retval = false;
-
-  if (cleanup_omp_return (bb))
-    return true;
-
-  retval = cleanup_control_flow_bb (bb);
+  bool retval = cleanup_control_flow_bb (bb);
 
   if (tree_forwarder_block_p (bb, false)
       && remove_forwarder_block (bb))
--- gcc/omp-low.c.jj	2012-07-16 14:38:18.000000000 +0200
+++ gcc/omp-low.c	2012-07-19 12:20:41.632384599 +0200
@@ -4751,45 +4751,40 @@  expand_omp_sections (struct omp_region *
   unsigned i, casei;
   bool exit_reachable = region->cont != NULL;
 
-  gcc_assert (exit_reachable == (region->exit != NULL));
+  gcc_assert (region->exit != NULL);
   entry_bb = region->entry;
   l0_bb = single_succ (entry_bb);
   l1_bb = region->cont;
   l2_bb = region->exit;
-  if (exit_reachable)
+  if (single_pred_p (l2_bb) && single_pred (l2_bb) == l0_bb)
+    l2 = gimple_block_label (l2_bb);
+  else
     {
-      if (single_pred_p (l2_bb) && single_pred (l2_bb) == l0_bb)
-	l2 = gimple_block_label (l2_bb);
+      /* This can happen if there are reductions.  */
+      len = EDGE_COUNT (l0_bb->succs);
+      gcc_assert (len > 0);
+      e = EDGE_SUCC (l0_bb, len - 1);
+      si = gsi_last_bb (e->dest);
+      l2 = NULL_TREE;
+      if (gsi_end_p (si)
+          || gimple_code (gsi_stmt (si)) != GIMPLE_OMP_SECTION)
+	l2 = gimple_block_label (e->dest);
       else
-	{
-	  /* This can happen if there are reductions.  */
-	  len = EDGE_COUNT (l0_bb->succs);
-	  gcc_assert (len > 0);
-	  e = EDGE_SUCC (l0_bb, len - 1);
-	  si = gsi_last_bb (e->dest);
-	  l2 = NULL_TREE;
-	  if (gsi_end_p (si)
-	      || gimple_code (gsi_stmt (si)) != GIMPLE_OMP_SECTION)
-	    l2 = gimple_block_label (e->dest);
-	  else
-	    FOR_EACH_EDGE (e, ei, l0_bb->succs)
+	FOR_EACH_EDGE (e, ei, l0_bb->succs)
+	  {
+	    si = gsi_last_bb (e->dest);
+	    if (gsi_end_p (si)
+		|| gimple_code (gsi_stmt (si)) != GIMPLE_OMP_SECTION)
 	      {
-		si = gsi_last_bb (e->dest);
-		if (gsi_end_p (si)
-		    || gimple_code (gsi_stmt (si)) != GIMPLE_OMP_SECTION)
-		  {
-		    l2 = gimple_block_label (e->dest);
-		    break;
-		  }
+		l2 = gimple_block_label (e->dest);
+		break;
 	      }
-	}
-      default_bb = create_empty_bb (l1_bb->prev_bb);
+	  }
     }
+  if (exit_reachable)
+    default_bb = create_empty_bb (l1_bb->prev_bb);
   else
-    {
-      default_bb = create_empty_bb (l0_bb);
-      l2 = gimple_block_label (default_bb);
-    }
+    default_bb = create_empty_bb (l0_bb);
 
   /* We will build a switch() with enough cases for all the
      GIMPLE_OMP_SECTION regions, a '0' case to handle the end of more work
@@ -4842,13 +4837,9 @@  expand_omp_sections (struct omp_region *
       vnext = NULL_TREE;
     }
 
-  i = 0;
-  if (exit_reachable)
-    {
-      t = build_case_label (build_int_cst (unsigned_type_node, 0), NULL, l2);
-      VEC_quick_push (tree, label_vec, t);
-      i++;
-    }
+  t = build_case_label (build_int_cst (unsigned_type_node, 0), NULL, l2);
+  VEC_quick_push (tree, label_vec, t);
+  i = 1;
 
   /* Convert each GIMPLE_OMP_SECTION into a CASE_LABEL_EXPR.  */
   for (inner = region->inner, casei = 1;
@@ -4918,18 +4909,18 @@  expand_omp_sections (struct omp_region *
       gsi_remove (&si, true);
 
       single_succ_edge (l1_bb)->flags = EDGE_FALLTHRU;
-
-      /* Cleanup function replaces GIMPLE_OMP_RETURN in EXIT_BB.  */
-      si = gsi_last_bb (l2_bb);
-      if (gimple_omp_return_nowait_p (gsi_stmt (si)))
-	t = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_END_NOWAIT);
-      else
-	t = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_END);
-      stmt = gimple_build_call (t, 0);
-      gsi_insert_after (&si, stmt, GSI_SAME_STMT);
-      gsi_remove (&si, true);
     }
 
+  /* Cleanup function replaces GIMPLE_OMP_RETURN in EXIT_BB.  */
+  si = gsi_last_bb (l2_bb);
+  if (gimple_omp_return_nowait_p (gsi_stmt (si)))
+    t = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_END_NOWAIT);
+  else
+    t = builtin_decl_explicit (BUILT_IN_GOMP_SECTIONS_END);
+  stmt = gimple_build_call (t, 0);
+  gsi_insert_after (&si, stmt, GSI_SAME_STMT);
+  gsi_remove (&si, true);
+
   set_immediate_dominator (CDI_DOMINATORS, default_bb, l0_bb);
 }
 
--- gcc/testsuite/c-c++-common/gomp/pr54017.c.jj	2012-07-19 12:46:22.749046807 +0200
+++ gcc/testsuite/c-c++-common/gomp/pr54017.c	2012-07-19 12:46:38.765960158 +0200
@@ -0,0 +1,65 @@ 
+/* PR middle-end/54017 */
+/* { dg-do compile } */
+/* { dg-options "-fopenmp" } */
+
+void
+f1 (void)
+{
+#pragma omp parallel sections
+  {
+#pragma omp section
+    {
+      for (;;)
+	;
+    }
+  }
+}
+
+int
+f2 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp sections reduction(+:i)
+  {
+#pragma omp section
+    {
+      for (;;)
+	;
+    }
+  }
+  return i;
+}
+
+void
+f3 (void)
+{
+#pragma omp parallel sections
+  {
+#pragma omp section
+    {
+      for (;;)
+	;
+    }
+#pragma omp section
+    ;
+  }
+}
+
+int
+f4 (void)
+{
+  int i = 0;
+#pragma omp parallel
+#pragma omp sections reduction(+:i)
+  {
+#pragma omp section
+    {
+      for (;;)
+	;
+    }
+#pragma omp section
+    ;
+  }
+  return i;
+}