diff mbox

[4/16] Implement -foffload-alias

Message ID 56E746A2.3000207@mentor.com
State New
Headers show

Commit Message

Tom de Vries March 14, 2016, 11:17 p.m. UTC
On 14/03/16 14:16, Tom de Vries wrote:
> On 02/12/15 10:58, Jakub Jelinek wrote:
>> On Fri, Nov 27, 2015 at 01:03:52PM +0100, Tom de Vries wrote:
>>> Handle non-declared variables in kernels alias analysis
>>>
>>> 2015-11-27  Tom de Vries  <tom@codesourcery.com>
>>>
>>>     * gimplify.c (gimplify_scan_omp_clauses): Initialize
>>>     OMP_CLAUSE_ORIG_DECL.
>>>     * omp-low.c (install_var_field_1): Handle base_pointers_restrict for
>>>     pointers.
>>>     (map_ptr_clause_points_to_clause_p)
>>>     (nr_map_ptr_clauses_pointing_to_clause): New function.
>>>     (omp_target_base_pointers_restrict_p): Handle GOMP_MAP_POINTER.
>>>     * tree-pretty-print.c (dump_omp_clause): Print OMP_CLAUSE_ORIG_DECL.
>>>     * tree.c (omp_clause_num_ops): Set num_ops for OMP_CLAUSE_MAP to 3.
>>>     * tree.h (OMP_CLAUSE_ORIG_DECL): New macro.
>>>
>>>     * c-c++-common/goacc/kernels-alias-10.c: New test.
>>>     * c-c++-common/goacc/kernels-alias-9.c: New test.
>>
>> I don't like this (mainly the addition of OMP_CLAUSE_ORIG_DECL),
>> but it also sounds wrong to me.
>> The primary question is how do you handle GOMP_MAP_POINTER
>> (which is something we don't use for C/C++ OpenMP anymore,
>> and Fortran OpenMP will stop using it in GCC 7 or 6.2?) on the OpenACC
>> libgomp side, does it work like GOMP_MAP_ALLOC or GOMP_MAP_FORCE_ALLOC?
>
> When a GOMP_MAP_POINTER mapping is encountered, first we check if it has
> been mapped before:
> - if it hasn't been mapped before, we check if the area the pointer
>    points to has been mapped, and if not, error out. Else we map the
>    pointer to a device pointer, and write the device pointer value
>    to the device pointer variable.
> - if the pointer has been mapped before, we reuse the mapping and write
>    the device pointer value to the device pointer variable.
>
>> Similarly GOMP_MAP_TO_PSET.
>> If it works like GOMP_MAP_ALLOC (it does
>> on the OpenMP side in target.c, so if something is already mapped, no
>> further pointer assignment happens), then your change looks wrong.
>> If it works like GOMP_MAP_FORCE_ALLOC, then you just should treat
>> GOMP_MAP_POINTER on all OpenACC constructs as opcode that allows the
>> restrict operation.
>
> I guess it works mostly like GOMP_MAP_ALLOC, but I don't understand the
> relevance of the comparison for the patch. What is interesting for the
> restrict optimization is whether what GOMP_MAP_POINTER points to has
> been mapped with or without the force flag during the same mapping
> sequence.
>
>> If it should behave differently depending on
>> if the corresponding array section has been mapped with GOMP_MAP_FORCE_*
>> or without it,
>
> The mapping itself shouldn't behave differently.
>
>> then supposedly you should use a different code for
>> those two.
>
> I could add f.i. an unsigned int aux_flags to struct tree_omp_clause,
> set a new POINTS_TO_FORCE_VAR flag when translating the acc clause into
> mapping clauses, and use that flag later on when dealing with the
> GOMP_MAP_POINTER clause. Is that an acceptable approach?
>
> [ Instead I could define a new gcc-internal-only
> GOMP_MAP_POINTER_POINTS_TO_FORCE kind, but I'd rather avoid this, given
> that it would be handled the same as GOMP_MAP_POINTER everywhere, except
> for a single point in the source code. ]

I found the example of OMP_CLAUSE_MAP_ZERO_BIAS_ARRAY_SECTION and 
OMP_CLAUSE_MAP_MAYBE_ZERO_LENGTH_ARRAY_SECTION, which re-purpose 
existing but unused fields, and used something similar in attached patch 
(untested, c-only for the moment).

Thanks,
- Tom
diff mbox

Patch

2016-03-14  Tom de Vries  <tom@codesourcery.com>

	* omp-low.c (install_var_field): Handle base_pointers_restrict for
	pointers.
	(omp_target_base_pointers_restrict_p): Handle GOMP_MAP_POINTER.
	* tree.h (OMP_CLAUSE_MAP_POINTER_TO_FORCED): define.

	* c-typeck.c (handle_omp_array_sections): Set
	OMP_CLAUSE_MAP_POINTER_TO_FORCED on GOMP_MAP_POINTER clause.

	* c-c++-common/goacc/kernels-alias-10.c: New test.
	* c-c++-common/goacc/kernels-alias-9.c: New test.

Handle non-declared variables in kernels alias analysis

---
 gcc/c/c-typeck.c                                   | 15 ++++++-
 gcc/omp-low.c                                      | 48 ++++++++++++++++++++++
 .../c-c++-common/goacc/kernels-alias-10.c          | 29 +++++++++++++
 gcc/testsuite/c-c++-common/goacc/kernels-alias-9.c | 29 +++++++++++++
 gcc/tree.h                                         |  3 ++
 5 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 6aa0f03..a05831d 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -12446,7 +12446,20 @@  handle_omp_array_sections (tree c, bool is_omp)
 	  }
       tree c2 = build_omp_clause (OMP_CLAUSE_LOCATION (c), OMP_CLAUSE_MAP);
       if (!is_omp)
-	OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_POINTER);
+	{
+	  OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_POINTER);
+	  switch (OMP_CLAUSE_MAP_KIND (c))
+	    {
+	    case GOMP_MAP_FORCE_ALLOC:
+	    case GOMP_MAP_FORCE_TO:
+	    case GOMP_MAP_FORCE_FROM:
+	    case GOMP_MAP_FORCE_TOFROM:
+	      OMP_CLAUSE_MAP_POINTER_TO_FORCED (c2) = 1;
+	      break;
+	    default:
+	      break;
+	    }
+	}
       else if (TREE_CODE (t) == COMPONENT_REF)
 	OMP_CLAUSE_SET_MAP_KIND (c2, GOMP_MAP_ALWAYS_POINTER);
       else
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 82dec9d..f9d953d 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -1429,6 +1429,9 @@  install_var_field (tree var, bool by_ref, int mask, omp_context *ctx,
     }
   else if (by_ref)
     {
+      if (base_pointers_restrict
+	  && POINTER_TYPE_P (type))
+	type = build_qualified_type (type, TYPE_QUAL_RESTRICT);
       type = build_pointer_type (type);
       if (base_pointers_restrict)
 	type = build_qualified_type (type, TYPE_QUAL_RESTRICT);
@@ -3132,6 +3135,47 @@  omp_target_base_pointers_restrict_p (tree clauses)
      Because both mappings have the force prefix, we know that they will be
      allocated when calling the corresponding offloaded function, which means we
      can mark the base pointers for a and b in the offloaded function as
+     restrict.
+
+     II.  GOMP_MAP_POINTER example:
+
+       void foo (unsigned int *a, unsigned int *b)
+       {
+	 #pragma acc kernels copyout (a[0:2]) copyout (b[0:2])
+	 {
+	   a[0] = 0;
+	   b[0] = 1;
+	 }
+       }
+
+     After gimplification, we have:
+
+     foo (unsigned int * a, unsigned int * b)
+     {
+       unsigned int * b.0;
+       unsigned int * a.1;
+
+       b.0 = b;
+       a.1 = a;
+       #pragma omp target oacc_kernels \
+	 map(force_from:*a.1 (*a) [len: 8]) \
+	 map(alloc:a [pointer assign, bias: 0]) \
+	 map(force_from:*b.0 (*b) [len: 8]) \
+	 map(alloc:b [pointer assign, bias: 0])
+       {
+	 unsigned int * a.2;
+	 unsigned int * b.3;
+
+	 a.2 = a;
+	 *a.2 = 0;
+	 b.3 = b;
+	 *b.3 = 1;
+       }
+     }
+
+     By testing for OMP_CLAUSE_MAP_POINTER_TO_FORCED, we can known for both
+     pointer assign mappings that they point to a force-prefixed mapping,  so
+     we can mark the base pointers for a and b in the offloaded function as
      restrict.  */
 
   tree c;
@@ -3147,6 +3191,10 @@  omp_target_base_pointers_restrict_p (tree clauses)
 	case GOMP_MAP_FORCE_FROM:
 	case GOMP_MAP_FORCE_TOFROM:
 	  break;
+	case GOMP_MAP_POINTER:
+	  if (!OMP_CLAUSE_MAP_POINTER_TO_FORCED (c))
+	    return false;
+	  break;
 	default:
 	  return false;
 	}
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-alias-10.c b/gcc/testsuite/c-c++-common/goacc/kernels-alias-10.c
new file mode 100644
index 0000000..ce5bbe8
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-alias-10.c
@@ -0,0 +1,29 @@ 
+/* { dg-additional-options "-O2" } */
+/* { dg-additional-options "-fdump-tree-ealias-all" } */
+
+#define N 2
+
+void
+foo (void)
+{
+  unsigned int a[N];
+  unsigned int b[N];
+  unsigned int c[N];
+  unsigned int d[N];
+
+#pragma acc kernels copyin (a[0:N]) create (b[0:N]) copyout (c[0:N]) copy (d[0:N])
+  {
+    a[0] = 0;
+    b[0] = 0;
+    c[0] = 0;
+    d[0] = 0;
+  }
+}
+
+/* { dg-final { scan-tree-dump-times "clique 1 base 1" 4 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 2" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 3" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 4" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 5" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "(?n)clique .* base .*" 8 "ealias" } } */
+
diff --git a/gcc/testsuite/c-c++-common/goacc/kernels-alias-9.c b/gcc/testsuite/c-c++-common/goacc/kernels-alias-9.c
new file mode 100644
index 0000000..7229fd4
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/kernels-alias-9.c
@@ -0,0 +1,29 @@ 
+/* { dg-additional-options "-O2" } */
+/* { dg-additional-options "-fdump-tree-ealias-all" } */
+
+#define N 2
+
+void
+foo (unsigned int *a, unsigned int *b, unsigned int *c, unsigned int *d)
+{
+
+#pragma acc kernels copyin (a[0:N]) create (b[0:N]) copyout (c[0:N]) copy (d[0:N])
+  {
+    a[0] = 0;
+    b[0] = 0;
+    c[0] = 0;
+    d[0] = 0;
+  }
+}
+
+/* { dg-final { scan-tree-dump-times "clique 1 base 1" 4 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 2" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 3" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 4" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 5" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 6" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 7" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 8" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "clique 1 base 9" 1 "ealias" } } */
+/* { dg-final { scan-tree-dump-times "(?n)clique .* base .*" 12 "ealias" } } */
+
diff --git a/gcc/tree.h b/gcc/tree.h
index 544a6a1..bc48ea8 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1533,6 +1533,9 @@  extern void protected_set_expr_location (tree, location_t);
 #define OMP_CLAUSE_MAP_MAYBE_ZERO_LENGTH_ARRAY_SECTION(NODE) \
   TREE_PROTECTED (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP))
 
+#define OMP_CLAUSE_MAP_POINTER_TO_FORCED(NODE) \
+  TREE_PRIVATE (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_MAP))
+
 #define OMP_CLAUSE_PROC_BIND_KIND(NODE) \
   (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_PROC_BIND)->omp_clause.subcode.proc_bind_kind)