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]

[PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687)


Hi!

If there are forced or non-local labels in OpenMP parallel/task/target
regions (or OpenACC or Cilk+), then we often fail to link as the following
testcases show - the labels aren't emitted anywhere, just references to
them.

While it is true that for forced/non-local labels we can't clone the basic
blocks containing them, for OpenMP we actually aren't cloning it, but
outlining the SESE region to a new function, i.e. moving the blocks.
As such, we can move the label too.  It is UB if a goto to such a label
(computed or non-local) is performed across boundaries of the region
(from outside of the region into it or from inside of it to outside),
but it should be ok to use it within the same region, and if just printing
address or something similar we can even reference label in one function
from another one (where one of them is outlined from the other or similar).

The following patch arranges for this - does not copy such labels during
lowering, and ensures that their DECL_CONTEXT will be the function that
contains the label (rather than just any reference to it).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-08-04  Jakub Jelinek  <jakub@redhat.com>

	PR c/81687
	* omp-low.c (omp_copy_decl): Don't remap FORCED_LABEL or DECL_NONLOCAL
	LABEL_DECLs.
	* tree-cfg.c (move_stmt_op): Don't adjust DECL_CONTEXT of FORCED_LABEL
	or DECL_NONLOCAL labels.
	(move_stmt_r) <case GIMPLE_LABEL>: Adjust DECL_CONTEXT of FORCED_LABEL
	or DECL_NONLOCAL labels here.

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

--- gcc/omp-low.c.jj	2017-08-03 10:33:41.000000000 +0200
+++ gcc/omp-low.c	2017-08-03 15:37:52.998873566 +0200
@@ -796,6 +796,8 @@ omp_copy_decl (tree var, copy_body_data
 
   if (TREE_CODE (var) == LABEL_DECL)
     {
+      if (FORCED_LABEL (var) || DECL_NONLOCAL (var))
+	return var;
       new_var = create_artificial_label (DECL_SOURCE_LOCATION (var));
       DECL_CONTEXT (new_var) = current_function_decl;
       insert_decl_map (&ctx->cb, var, new_var);
--- gcc/tree-cfg.c.jj	2017-07-29 09:48:43.000000000 +0200
+++ gcc/tree-cfg.c	2017-08-03 15:37:40.395021370 +0200
@@ -6718,7 +6718,15 @@ move_stmt_op (tree *tp, int *walk_subtre
 		*tp = t = out->to;
 	    }
 
-	  DECL_CONTEXT (t) = p->to_context;
+	  /* For FORCED_LABELs we can end up with references from other
+	     functions if some SESE regions are outlined.  It is UB to
+	     jump in between them, but they could be used just for printing
+	     addresses etc.  In that case, DECL_CONTEXT on the label should
+	     be the function containing the glabel stmt with that LABEL_DECL,
+	     rather than whatever function a reference to the label was seen
+	     last time.  */
+	  if (!FORCED_LABEL (t) && !DECL_NONLOCAL (t))
+	    DECL_CONTEXT (t) = p->to_context;
 	}
       else if (p->remap_decls_p)
 	{
@@ -6836,6 +6844,21 @@ move_stmt_r (gimple_stmt_iterator *gsi_p
     case GIMPLE_OMP_RETURN:
     case GIMPLE_OMP_CONTINUE:
       break;
+
+    case GIMPLE_LABEL:
+      {
+	/* For FORCED_LABEL, move_stmt_op doesn't adjust DECL_CONTEXT,
+	   so that such labels can be referenced from other regions.
+	   Make sure to update it when seeing a GIMPLE_LABEL though,
+	   that is the owner of the label.  */
+	walk_gimple_op (stmt, move_stmt_op, wi);
+	*handled_ops_p = true;
+	tree label = gimple_label_label (as_a <glabel *> (stmt));
+	if (FORCED_LABEL (label) || DECL_NONLOCAL (label))
+	  DECL_CONTEXT (label) = p->to_context;
+      }
+      break;
+
     default:
       if (is_gimple_omp (stmt))
 	{
--- libgomp/testsuite/libgomp.c/pr81687-1.c.jj	2017-08-03 15:43:32.832888380 +0200
+++ libgomp/testsuite/libgomp.c/pr81687-1.c	2017-08-03 15:43:06.000000000 +0200
@@ -0,0 +1,23 @@
+/* PR c/81687 */
+/* { dg-do link } */
+/* { dg-additional-options "-O2" } */
+
+extern int printf (const char *, ...);
+
+int
+main ()
+{
+  #pragma omp parallel
+  {
+   lab1:
+    printf ("lab1=%p\n", (void *)(&&lab1));
+  }
+ lab2:
+  #pragma omp parallel
+  {
+   lab3:
+    printf ("lab2=%p\n", (void *)(&&lab2));
+  }
+  printf ("lab3=%p\n", (void *)(&&lab3));
+  return 0;
+}
--- libgomp/testsuite/libgomp.c/pr81687-2.c.jj	2017-08-03 15:43:36.229848545 +0200
+++ libgomp/testsuite/libgomp.c/pr81687-2.c	2017-08-03 15:43:15.000000000 +0200
@@ -0,0 +1,27 @@
+/* PR c/81687 */
+/* { dg-do link } */
+/* { dg-additional-options "-O2" } */
+
+int
+main ()
+{
+  __label__ lab4, lab5, lab6;
+  volatile int l = 0;
+  int m = l;
+  void foo (int x) { if (x == 1) goto lab4; }
+  void bar (int x) { if (x == 2) goto lab5; }
+  void baz (int x) { if (x == 3) goto lab6; }
+  #pragma omp parallel
+  {
+    foo (m + 1);
+   lab4:;
+  }
+  #pragma omp task
+  {
+    bar (m + 2);
+   lab5:;
+  }
+  baz (m + 3);
+ lab6:;
+  return 0;
+}

	Jakub


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