On Fri, 4 Aug 2017, Jakub Jelinek wrote: > 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?
Ok. Thanks, Richard. > 2017-08-04 Jakub Jelinek <ja...@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 > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)