diff mbox series

[committed] openmp: Fix up handling of doacross loops with noreturn body in loops [PR108685]

Message ID ZBQe+haaKNvzZMw5@tucnak
State New
Headers show
Series [committed] openmp: Fix up handling of doacross loops with noreturn body in loops [PR108685] | expand

Commit Message

Jakub Jelinek March 17, 2023, 8:04 a.m. UTC 
Hi!

The following patch fixes an ICE with doacross loops which have a single entry
no exit body, at least one of the ordered > collapse loops isn't guaranteed to
have at least one iteration and the whole doacross loop is inside some other loop.
The OpenMP constructs aren't represented by struct loop until the omp expansions,
so for a normal doacross loop which doesn't have a noreturn body the entry_bb
with the GOMP_FOR statement and the first bb of the body typically have the
same loop_father, and if the doacross loop isn't inside of some other loop
and the body is noreturn as well, both are part of loop 0.  The problematic
case is when the entry_bb is inside of some deeper loop, but the body, because
it falls through into EXIT, has loop 0 as loop_father.  l0_bb is created by
splitting the entry_bb fallthru edge into l1_bb, and because the two basic blocks
have different loop_father, a common loop is found for those (which is loop 0).
Now, if the doacross loop has collapse == ordered or all the ordered > collapse
loops are guaranteed to iterate at least once, all is still fine, because all
enter the l1_bb (body), which doesn't return and so doesn't loop further either.
But, if one of those loops could loop 0 times, the user written body wouldn't be
reached at all, so unlike the expectations the whole construct actually wouldn't
be noreturn if entry_bb is encountered and decides to handle at least one
iteration.

In this case, we need to fix up, move the l0_bb into the same loop as entry_bb
(initially) and for the extra added loops put them as children of that same
loop, rather than of loop 0.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.

2023-03-17  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/108685
	* omp-expand.cc (expand_omp_for_ordered_loops): Add L0_BB argument,
	use its loop_father rather than BODY_BB's loop_father.
	(expand_omp_for_generic): Adjust expand_omp_for_ordered_loops caller.
	If broken_loop with ordered > collapse and at least one of those
	extra loops aren't guaranteed to have at least one iteration, change
	l0_bb's loop_father to entry_bb's loop_father.  Set cont_bb's
	loop_father to l0_bb's loop_father rather than l1_bb's.

	* c-c++-common/gomp/doacross-8.c: New test.


	Jakub
diff mbox series

Patch

--- gcc/omp-expand.cc.jj	2023-02-16 10:13:01.000000000 +0100
+++ gcc/omp-expand.cc	2023-03-16 17:19:46.180181287 +0100
@@ -3674,7 +3674,7 @@  expand_omp_ordered_source_sink (struct o
 static basic_block
 expand_omp_for_ordered_loops (struct omp_for_data *fd, tree *counts,
 			      basic_block cont_bb, basic_block body_bb,
-			      bool ordered_lastprivate)
+			      basic_block l0_bb, bool ordered_lastprivate)
 {
   if (fd->ordered == fd->collapse)
     return cont_bb;
@@ -3783,7 +3783,7 @@  expand_omp_for_ordered_loops (struct omp
 	  class loop *loop = alloc_loop ();
 	  loop->header = new_header;
 	  loop->latch = e2->src;
-	  add_loop (loop, body_bb->loop_father);
+	  add_loop (loop, l0_bb->loop_father);
 	}
     }
 
@@ -4481,9 +4481,15 @@  expand_omp_for_generic (struct omp_regio
 	    }
 	  if (i < fd->ordered)
 	    {
+	      if (entry_bb->loop_father != l0_bb->loop_father)
+		{
+		  remove_bb_from_loops (l0_bb);
+		  add_bb_to_loop (l0_bb, entry_bb->loop_father);
+		  gcc_assert (single_succ (l0_bb) == l1_bb);
+		}
 	      cont_bb
 		= create_empty_bb (EXIT_BLOCK_PTR_FOR_FN (cfun)->prev_bb);
-	      add_bb_to_loop (cont_bb, l1_bb->loop_father);
+	      add_bb_to_loop (cont_bb, l0_bb->loop_father);
 	      gimple_stmt_iterator gsi = gsi_after_labels (cont_bb);
 	      gimple *g = gimple_build_omp_continue (fd->loop.v, fd->loop.v);
 	      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
@@ -4495,7 +4501,7 @@  expand_omp_for_generic (struct omp_regio
 	}
       expand_omp_ordered_source_sink (region, fd, counts, cont_bb);
       cont_bb = expand_omp_for_ordered_loops (fd, counts, cont_bb, l1_bb,
-					      ordered_lastprivate);
+					      l0_bb, ordered_lastprivate);
       if (counts[fd->collapse - 1])
 	{
 	  gcc_assert (fd->collapse == 1);
--- gcc/testsuite/c-c++-common/gomp/doacross-8.c.jj	2023-03-16 17:57:32.070167415 +0100
+++ gcc/testsuite/c-c++-common/gomp/doacross-8.c	2023-03-16 17:56:21.313199748 +0100
@@ -0,0 +1,17 @@ 
+/* PR middle-end/108685 */
+/* { dg-do compile } */
+
+void
+foo (int a)
+{
+  for (int m = 0; m < 10; m++)
+    #pragma omp for collapse(2) ordered(4)
+    for (int i = 0; i < 2; i++)
+      for (int j = 0; j < a; j++)
+	for (int k = 0; k < 2; k++)
+	  for (int l = 0; l < a; l++)
+	    {
+	      #pragma omp ordered depend (source)
+	      __builtin_abort ();
+	    }
+}