* [PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687)
@ 2017-08-04 15:34 Jakub Jelinek
2017-08-08 10:26 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2017-08-04 15:34 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687)
2017-08-04 15:34 [PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687) Jakub Jelinek
@ 2017-08-08 10:26 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-08-08 10:26 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
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 <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
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-08-08 10:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 15:34 [PATCH] Fix OpenMP/OpenACC/Cilk+ issues with forced/non-local labels (PR c/81687) Jakub Jelinek
2017-08-08 10:26 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).