Patchwork [committed] Fix OpenMP shared clause handling of DECL_BY_REFERENCE RESULT_DECLs/PARM_DECLs (PR middle-end/56217)

login
register
mail settings
Submitter Jakub Jelinek
Date Feb. 6, 2013, 10:41 a.m.
Message ID <20130206104127.GR4385@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/218537/
State New
Headers show

Comments

Jakub Jelinek - Feb. 6, 2013, 10:41 a.m.
Hi!

use_pointer_for_field in some cases returns true to avoid copy-in/out,
which results in the address of the decl being passed around instead of
the decl itself.
But for RESULT_DECL or PARM_DECL of REFERENCE_TYPE we generate only
copy-in, not copy-out (as references can't change), so there is no point
passing it as pointer to the reference to something, the reference can't
change, only what it references.  Besides this generating better code,
making RESULT_DECL of REFERENCE_TYPE addressable and regimplifying it
can result in invalid IL.

Fixed thusly, tested on x86_64-linux, committed to trunk (4.7 backport will
follow later on).

2013-02-06  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/56217
	* omp-low.c (use_pointer_for_field): Return false if
	lower_send_shared_vars doesn't generate any copy-out code.

	* g++.dg/gomp/pr56217.C: New test.

	* testsuite/libgomp.c++/pr56217.C: New test.


	Jakub

Patch

--- gcc/omp-low.c.jj	2013-01-11 09:02:35.000000000 +0100
+++ gcc/omp-low.c	2013-02-06 10:42:19.016346866 +0100
@@ -757,12 +757,20 @@  use_pointer_for_field (tree decl, omp_co
       if (TREE_ADDRESSABLE (decl))
 	return true;
 
+      /* lower_send_shared_vars only uses copy-in, but not copy-out
+	 for these.  */
+      if (TREE_READONLY (decl)
+	  || ((TREE_CODE (decl) == RESULT_DECL
+	       || TREE_CODE (decl) == PARM_DECL)
+	      && DECL_BY_REFERENCE (decl)))
+	return false;
+
       /* Disallow copy-in/out in nested parallel if
 	 decl is shared in outer parallel, otherwise
 	 each thread could store the shared variable
 	 in its own copy-in location, making the
 	 variable no longer really shared.  */
-      if (!TREE_READONLY (decl) && shared_ctx->is_nested)
+      if (shared_ctx->is_nested)
 	{
 	  omp_context *up;
 
@@ -785,11 +793,10 @@  use_pointer_for_field (tree decl, omp_co
 	    }
 	}
 
-      /* For tasks avoid using copy-in/out, unless they are readonly
-	 (in which case just copy-in is used).  As tasks can be
+      /* For tasks avoid using copy-in/out.  As tasks can be
 	 deferred or executed in different thread, when GOMP_task
 	 returns, the task hasn't necessarily terminated.  */
-      if (!TREE_READONLY (decl) && is_task_ctx (shared_ctx))
+      if (is_task_ctx (shared_ctx))
 	{
 	  tree outer;
 	maybe_mark_addressable_and_ret:
--- gcc/testsuite/g++.dg/gomp/pr56217.C.jj	2013-02-06 10:44:19.001660809 +0100
+++ gcc/testsuite/g++.dg/gomp/pr56217.C	2013-02-06 10:45:56.991115914 +0100
@@ -0,0 +1,14 @@ 
+// PR middle-end/56217
+// { dg-do compile }
+// { dg-options "-fopenmp" }
+
+struct S { int *p; S (); S (S &); };
+
+S
+foo ()
+{
+  S s;
+  #pragma omp task shared (s)
+    s.p = 0;
+  return s;
+}
--- libgomp/testsuite/libgomp.c++/pr56217.C.jj	2013-02-06 10:48:10.125404318 +0100
+++ libgomp/testsuite/libgomp.c++/pr56217.C	2013-02-06 11:16:30.000000000 +0100
@@ -0,0 +1,36 @@ 
+// PR middle-end/56217
+// { dg-do run }
+// { dg-options "-std=c++0x" }
+
+extern "C" void abort ();
+
+template <typename T>
+struct ptr {
+  T *p;
+  ptr () : p () {}
+  ptr (ptr &) = delete;
+  ptr (ptr &&o) : p(o) {}
+  operator T * () { return p; }
+};
+
+int a[6] = { 100, 101, 102, 103, 104, 105 };
+
+static ptr<int>
+f ()
+{
+  ptr<int> pt;
+  #pragma omp task shared (pt)
+    pt.p = a + 2;
+  #pragma omp taskwait
+  return pt;
+}
+
+int
+main ()
+{
+  ptr<int> pt;
+  #pragma omp parallel
+  #pragma omp single
+  if (f () != a + 2 || *f () != 102)
+    abort ();
+}