diff mbox

[committed] Fix OpenMP VLA firstprivatization on task (PR middle-end/80809)

Message ID 20170522190655.GH8499@tucnak
State New
Headers show

Commit Message

Jakub Jelinek May 22, 2017, 7:06 p.m. UTC 
Hi!

This PR shows another bug, the computation of the size argument
for GOMP_task call could include uninitialized variable uses.

The problem is that the task arg_size needs to be finalized only
after mwe return from omp_low_1 of the task construct, thus it
is hard to gimplify it somewhere, but it can include references
to temporaries that need to be remapped if in nested constructs.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk, queued for release branches.

2017-05-22  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/80809
	* omp-low.c (finish_taskreg_remap): New function.
	(finish_taskreg_scan): If unit size of ctx->record_type
	is non-constant, unshare the size expression and replace
	decls in it with possible outer var refs.

	* testsuite/libgomp.c/pr80809-2.c: New test.
	* testsuite/libgomp.c/pr80809-3.c: New test.


	Jakub
diff mbox

Patch

--- gcc/omp-low.c.jj	2017-05-22 11:34:51.000000000 +0200
+++ gcc/omp-low.c	2017-05-22 18:31:30.045010460 +0200
@@ -1913,6 +1913,29 @@  scan_omp_task (gimple_stmt_iterator *gsi
     }
 }
 
+/* Helper function for finish_taskreg_scan, called through walk_tree.
+   If maybe_lookup_decl_in_outer_context returns non-NULL for some
+   tree, replace it in the expression.  */
+
+static tree
+finish_taskreg_remap (tree *tp, int *walk_subtrees, void *data)
+{
+  if (VAR_P (*tp))
+    {
+      omp_context *ctx = (omp_context *) data;
+      tree t = maybe_lookup_decl_in_outer_ctx (*tp, ctx);
+      if (t != *tp)
+	{
+	  if (DECL_HAS_VALUE_EXPR_P (t))
+	    t = unshare_expr (DECL_VALUE_EXPR (t));
+	  *tp = t;
+	}
+      *walk_subtrees = 0;
+    }
+  else if (IS_TYPE_OR_DECL_P (*tp))
+    *walk_subtrees = 0;
+  return NULL_TREE;
+}
 
 /* If any decls have been made addressable during scan_omp,
    adjust their fields if needed, and layout record types
@@ -2033,6 +2056,11 @@  finish_taskreg_scan (omp_context *ctx)
 	layout_type (ctx->srecord_type);
       tree t = fold_convert_loc (loc, long_integer_type_node,
 				 TYPE_SIZE_UNIT (ctx->record_type));
+      if (TREE_CODE (t) != INTEGER_CST)
+	{
+	  t = unshare_expr (t);
+	  walk_tree (&t, finish_taskreg_remap, ctx, NULL);
+	}
       gimple_omp_task_set_arg_size (ctx->stmt, t);
       t = build_int_cst (long_integer_type_node,
 			 TYPE_ALIGN_UNIT (ctx->record_type));
--- libgomp/testsuite/libgomp.c/pr80809-2.c.jj	2017-05-22 18:36:28.950298209 +0200
+++ libgomp/testsuite/libgomp.c/pr80809-2.c	2017-05-22 14:28:40.000000000 +0200
@@ -0,0 +1,35 @@ 
+/* PR middle-end/80809 */
+/* { dg-do run } */
+
+__attribute__((noinline, noclone)) void
+foo (int x)
+{
+  int i, v[x], w[16];
+  for (i = 0; i < x; i++)
+    v[i] = i;
+  for (i = 0; i < 16; i++)
+    w[i] = 0;
+#pragma omp parallel
+#pragma omp single
+  for (i = 0; i < 16; i++)
+#pragma omp task firstprivate (v)
+    {
+      int j;
+      for (j = 0; j < x; j++)
+	v[j] += i;
+      for (j = 0; j < x; j++)
+	w[i] += v[j];
+    }
+  for (i = 0; i < 16; i++)
+    if (w[i] != (x - 1) * x / 2 + x * i)
+      __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo (4);
+  foo (27);
+  foo (196);
+  return 0;
+}
--- libgomp/testsuite/libgomp.c/pr80809-3.c.jj	2017-05-22 18:36:32.252257201 +0200
+++ libgomp/testsuite/libgomp.c/pr80809-3.c	2017-05-22 18:34:46.000000000 +0200
@@ -0,0 +1,42 @@ 
+/* PR middle-end/80809 */
+/* { dg-do run } */
+
+__attribute__((noinline, noclone)) void
+foo (int x)
+{
+  int i, v[x], w[16];
+  for (i = 0; i < x; i++)
+    v[i] = i;
+  for (i = 0; i < 16; i++)
+    w[i] = 0;
+#pragma omp parallel
+#pragma omp single
+  {
+    int z[x];
+    for (i = 0; i < x; i++)
+      z[0] = 0;
+    for (i = 0; i < 16; i++)
+#pragma omp task firstprivate (z) firstprivate (v)
+      {
+	int j;
+	for (j = 0; j < x; j++)
+	  z[j] = i;
+	for (j = 0; j < x; j++)
+	  v[j] += z[j];
+	for (j = 0; j < x; j++)
+	  w[i] += v[j];
+      }
+  }
+  for (i = 0; i < 16; i++)
+    if (w[i] != (x - 1) * x / 2 + x * i)
+      __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo (4);
+  foo (27);
+  foo (196);
+  return 0;
+}