diff mbox

[gomp4.1] calculate pointer offsets for depend(sink)

Message ID 55AECD7B.6040709@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez July 21, 2015, 10:53 p.m. UTC 
On 07/21/2015 09:30 AM, Jakub Jelinek wrote:
> On Tue, Jul 21, 2015 at 09:08:27AM -0700, Aldy Hernandez wrote:
>> --- a/gcc/c/c-typeck.c
>> +++ b/gcc/c/c-typeck.c
>> @@ -12492,6 +12492,24 @@ c_finish_omp_clauses (tree clauses, bool declare_simd)
>>   	  if (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
>>   	    {
>>   	      gcc_assert (TREE_CODE (t) == TREE_LIST);
>> +	      for (; t; t = TREE_CHAIN (t))
>> +		{
>> +		  tree decl = TREE_VALUE (t);
>> +		  if (TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE)
>> +		    {
>> +		      tree offset = TREE_PURPOSE (t);
>> +		      bool neg = wi::neg_p ((wide_int) offset);
>> +		      offset = fold_unary (ABS_EXPR, TREE_TYPE (offset), offset);
>> +		      tree t2 = pointer_int_sum (OMP_CLAUSE_LOCATION (c),
>> +						 neg ? MINUS_EXPR : PLUS_EXPR,
>> +						 decl, offset);
>> +		      t2 = fold_build2_loc (OMP_CLAUSE_LOCATION (c), MINUS_EXPR,
>> +					    sizetype, t2, decl);
>> +		      if (t2 == error_mark_node)
>> +			t2 = size_one_node; // ??
>
> I think we should just remove the whole OMP_CLAUSE_DEPEND_SINK clause
> in case of errors.

That was my first idea, but then I saw the discrepancy in the C and C++ 
FE's handling OMP_CLAUSE_LINEAR_STEP.  The C FE has:

	      if (s == error_mark_node)
		s = size_one_node;

whereas the C++ FE has:

		      if (t == error_mark_node)
			{
			  remove = true;
			  break;
			}

However, I have removed the clause as suggested.

>
>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
>> index 8c05936..2598433 100644
>> --- a/gcc/cp/semantics.c
>> +++ b/gcc/cp/semantics.c
>> @@ -5532,6 +5532,41 @@ finish_omp_declare_simd_methods (tree t)
>>       }
>>   }
>>
>> +/* Adjust sink depend clause to take into account pointer offsets.  */
>> +
>> +static void
>> +cp_finish_omp_clause_depend_sink (tree sink_clause)
>> +{
>> +  tree t = OMP_CLAUSE_DECL (sink_clause);
>> +  gcc_assert (TREE_CODE (t) == TREE_LIST);
>> +
>> +  /* Make sure we don't adjust things twice for templates.  */
>> +  if (processing_template_decl)
>> +    return;
>> +
>> +  for (; t; t = TREE_CHAIN (t))
>> +    {
>> +      tree decl = TREE_VALUE (t);
>> +      if (TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE)
>> +	{
>> +	  tree offset = TREE_PURPOSE (t);
>> +	  bool neg = wi::neg_p ((wide_int) offset);
>> +	  offset = fold_unary (ABS_EXPR, TREE_TYPE (offset), offset);
>> +	  decl = mark_rvalue_use (decl);
>> +	  decl = convert_from_reference (decl);
>> +	  tree t2 = pointer_int_sum (OMP_CLAUSE_LOCATION (sink_clause),
>> +				     neg ? MINUS_EXPR : PLUS_EXPR,
>> +				     decl, offset);
>> +	  t2 = fold_build2_loc (OMP_CLAUSE_LOCATION (sink_clause),
>> +				MINUS_EXPR, sizetype, t2,
>> +				decl);
>> +	  if (t2 == error_mark_node)
>> +	    t2 = size_one_node; // ??
>
> Likewise.  But then the question is if it is worth to handle
> this in a separate routine, rather in the finish_omp_clauses function.

I still like the separate function. :)

>
>> +	  TREE_PURPOSE (t) = t2;
>> +	}
>> +    }
>> +}
>
>
>> +
>>   /* For all elements of CLAUSES, validate them vs OpenMP constraints.
>>      Remove any elements from the list that are invalid.  */
>>
>> @@ -6150,7 +6185,7 @@ finish_omp_clauses (tree clauses, bool allow_fields, bool declare_simd)
>>   	    }
>>   	  if (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
>>   	    {
>> -	      gcc_assert (TREE_CODE (t) == TREE_LIST);
>> +	      cp_finish_omp_clause_depend_sink (c);
>>   	      break;
>>   	    }
>>   	  if (TREE_CODE (t) == TREE_LIST)
>> diff --git a/gcc/testsuite/c-c++-common/gomp/sink-4.c b/gcc/testsuite/c-c++-common/gomp/sink-4.c
>> new file mode 100644
>> index 0000000..2872404
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/gomp/sink-4.c
>> @@ -0,0 +1,27 @@
>> +/* { dg-do compile } */
>> +/* { dg-options "-fopenmp -fdump-tree-gimple" } */
>> +
>> +/* Test that we adjust pointer offsets for sink variables
>> +   correctly.  */
>> +
>> +typedef struct {
>> +    char stuff[400];
>> +} foo;
>> +
>> +foo *p, *q;
>> +
>> +void
>> +funk ()
>> +{
>> +  int i,j;
>
> Unused vars?  Also, I'd suggest just pass foo *beg, foo *end
> arguments.

ughh, you and your politically correct testcases ;-).  Fixed.

How about the attached?
commit 61b2d11dfa8083014b385fc6ec6564fc18c41c72
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jul 21 08:02:39 2015 -0700

    	* tree-pretty-print.c (dump_omp_clause): Pass TYPE_SIGN to
    	wi::neg_p.
    c/
    	* c-typeck.c (c_finish_omp_clauses): Adjust pointer offsets for
    	OMP_CLAUSE_DEPEND_SINK.
    cp/
    	* semantics.c (cp_finish_omp_clause_depend_sink): New.
    	(finish_omp_clauses): Call cp_finish_omp_clause_depend_sink.

Comments

Jakub Jelinek July 22, 2015, 2:34 p.m. UTC | #1 
On Tue, Jul 21, 2015 at 03:53:47PM -0700, Aldy Hernandez wrote:
> commit 61b2d11dfa8083014b385fc6ec6564fc18c41c72
> Author: Aldy Hernandez <aldyh@redhat.com>
> Date:   Tue Jul 21 08:02:39 2015 -0700
> 
>     	* tree-pretty-print.c (dump_omp_clause): Pass TYPE_SIGN to
>     	wi::neg_p.
>     c/
>     	* c-typeck.c (c_finish_omp_clauses): Adjust pointer offsets for
>     	OMP_CLAUSE_DEPEND_SINK.
>     cp/
>     	* semantics.c (cp_finish_omp_clause_depend_sink): New.
>     	(finish_omp_clauses): Call cp_finish_omp_clause_depend_sink.

Ok, thanks.

	Jakub
diff mbox

Patch

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index b7714e3..b1d36dd 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -12492,6 +12492,27 @@  c_finish_omp_clauses (tree clauses, bool declare_simd)
 	  if (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
 	    {
 	      gcc_assert (TREE_CODE (t) == TREE_LIST);
+	      for (; t; t = TREE_CHAIN (t))
+		{
+		  tree decl = TREE_VALUE (t);
+		  if (TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE)
+		    {
+		      tree offset = TREE_PURPOSE (t);
+		      bool neg = wi::neg_p ((wide_int) offset);
+		      offset = fold_unary (ABS_EXPR, TREE_TYPE (offset), offset);
+		      tree t2 = pointer_int_sum (OMP_CLAUSE_LOCATION (c),
+						 neg ? MINUS_EXPR : PLUS_EXPR,
+						 decl, offset);
+		      t2 = fold_build2_loc (OMP_CLAUSE_LOCATION (c), MINUS_EXPR,
+					    sizetype, t2, decl);
+		      if (t2 == error_mark_node)
+			{
+			  remove = true;
+			  break;
+			}
+		      TREE_PURPOSE (t) = t2;
+		    }
+		}
 	      break;
 	    }
 	  if (TREE_CODE (t) == TREE_LIST)
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 8c05936..afb96f5 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5532,6 +5532,45 @@  finish_omp_declare_simd_methods (tree t)
     }
 }
 
+/* Adjust sink depend clause to take into account pointer offsets.
+
+   Return TRUE if there was a problem processing the offset, and the
+   whole clause should be removed.  */
+
+static bool
+cp_finish_omp_clause_depend_sink (tree sink_clause)
+{
+  tree t = OMP_CLAUSE_DECL (sink_clause);
+  gcc_assert (TREE_CODE (t) == TREE_LIST);
+
+  /* Make sure we don't adjust things twice for templates.  */
+  if (processing_template_decl)
+    return false;
+
+  for (; t; t = TREE_CHAIN (t))
+    {
+      tree decl = TREE_VALUE (t);
+      if (TREE_CODE (TREE_TYPE (decl)) == POINTER_TYPE)
+	{
+	  tree offset = TREE_PURPOSE (t);
+	  bool neg = wi::neg_p ((wide_int) offset);
+	  offset = fold_unary (ABS_EXPR, TREE_TYPE (offset), offset);
+	  decl = mark_rvalue_use (decl);
+	  decl = convert_from_reference (decl);
+	  tree t2 = pointer_int_sum (OMP_CLAUSE_LOCATION (sink_clause),
+				     neg ? MINUS_EXPR : PLUS_EXPR,
+				     decl, offset);
+	  t2 = fold_build2_loc (OMP_CLAUSE_LOCATION (sink_clause),
+				MINUS_EXPR, sizetype, t2,
+				decl);
+	  if (t2 == error_mark_node)
+	    return true;
+	  TREE_PURPOSE (t) = t2;
+	}
+    }
+  return false;
+}
+
 /* For all elements of CLAUSES, validate them vs OpenMP constraints.
    Remove any elements from the list that are invalid.  */
 
@@ -6150,7 +6189,8 @@  finish_omp_clauses (tree clauses, bool allow_fields, bool declare_simd)
 	    }
 	  if (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
 	    {
-	      gcc_assert (TREE_CODE (t) == TREE_LIST);
+	      if (cp_finish_omp_clause_depend_sink (c))
+		remove = true;
 	      break;
 	    }
 	  if (TREE_CODE (t) == TREE_LIST)
diff --git a/gcc/testsuite/c-c++-common/gomp/sink-4.c b/gcc/testsuite/c-c++-common/gomp/sink-4.c
new file mode 100644
index 0000000..7934de2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/sink-4.c
@@ -0,0 +1,25 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fopenmp -fdump-tree-gimple" } */
+
+/* Test that we adjust pointer offsets for sink variables
+   correctly.  */
+
+typedef struct {
+    char stuff[400];
+} foo;
+
+void
+funk (foo *begin, foo *end)
+{
+  foo *p;
+#pragma omp parallel for ordered(1)
+  for (p=end; p > begin; p--)
+    {
+#pragma omp ordered depend(sink:p+1)
+      void bar ();
+        bar();
+#pragma omp ordered depend(source)
+    }
+}
+
+/* { dg-final { scan-tree-dump-times "depend\\(sink:p\\+400\\)" 1 "gimple" } } */
diff --git a/gcc/testsuite/g++.dg/gomp/sink-3.C b/gcc/testsuite/g++.dg/gomp/sink-3.C
new file mode 100644
index 0000000..83a742e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gomp/sink-3.C
@@ -0,0 +1,33 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fopenmp -fdump-tree-gimple" } */
+
+/* Test that we adjust pointer offsets for sink variables
+   correctly.  */
+
+typedef struct {
+    char stuff[400];
+} foo;
+
+foo *p, *q, *r;
+
+template<int N>
+void
+funk ()
+{
+  int i,j;
+#pragma omp parallel for ordered(1)
+  for (p=q; p < q; p--)
+    {
+#pragma omp ordered depend(sink:p+1)
+      void bar ();
+        bar();
+#pragma omp ordered depend(source)
+    }
+}
+
+void foobar()
+{
+  funk<3>();
+}
+
+/* { dg-final { scan-tree-dump-times "depend\\(sink:p\\+400\\)" 1 "gimple" } } */
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index d3cc245..aab2bfc 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -567,7 +567,8 @@  dump_omp_clause (pretty_printer *pp, tree clause, int spc, int flags)
 		dump_generic_node (pp, TREE_VALUE (t), spc, flags, false);
 		if (TREE_PURPOSE (t) != integer_zero_node)
 		  {
-		    if (!wi::neg_p (TREE_PURPOSE (t)))
+		    tree p = TREE_PURPOSE (t);
+		    if (!wi::neg_p (p, TYPE_SIGN (TREE_TYPE (p))))
 		      pp_plus (pp);
 		    dump_generic_node (pp, TREE_PURPOSE (t), spc, flags,
 				       false);