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)
