subject:"\[gomp4\] add support for derived types in ACC UPDATE"

Re: [gomp4] add support for derived types in ACC UPDATE

2017-02-01 Thread Cesar Philippidis

On 01/30/2017 02:08 AM, Thomas Schwinge wrote:
> Hi Cesar!
> 
> On Thu, 10 Nov 2016 09:38:33 -0800, Cesar Philippidis 
>  wrote:
>> This patch has been committed to gomp-4_0-branch.
> 
>> --- a/gcc/fortran/openmp.c
>> +++ b/gcc/fortran/openmp.c
> 
>> @@ -242,7 +243,8 @@ gfc_match_omp_variable_list (const char *str, 
>> gfc_omp_namelist **list,
>>  case MATCH_YES:
>>gfc_expr *expr;
>>expr = NULL;
>> -  if (allow_sections && gfc_peek_ascii_char () == '(')
>> +  if (allow_sections && gfc_peek_ascii_char () == '('
>> +  || allow_derived && gfc_peek_ascii_char () == '%')
>>  {
>>gfc_current_locus = cur_loc;
>>m = gfc_match_variable (, 0);
> 
> [...]/source-gcc/gcc/fortran/openmp.c: In function 'match 
> {+gfc_match_omp_variable_list(const char*, gfc_omp_namelist**, bool, bool*, 
> gfc_omp_namelist***, bool, bool)':+}
> [...]/source-gcc/gcc/fortran/openmp.c:246:23: warning: suggest 
> parentheses around '&&' within '||' [-Wparentheses]
> if (allow_sections && gfc_peek_ascii_char () == '('
>^
> 
>> --- a/gcc/fortran/trans-openmp.c
>> +++ b/gcc/fortran/trans-openmp.c
>> @@ -1938,7 +1938,66 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, 
>> gfc_omp_clauses *clauses,
>>tree decl = gfc_get_symbol_decl (n->sym);
>>if (DECL_P (decl))
>>  TREE_ADDRESSABLE (decl) = 1;
>> -  if (n->expr == NULL || n->expr->ref->u.ar.type == AR_FULL)
>> +  /* Handle derived-typed members for OpenACC Update.  */
>> +  if (n->sym->ts.type == BT_DERIVED
>> +  && n->expr != NULL && n->expr->ref != NULL
>> +  && (n->expr->ref->next == NULL
>> +  || (n->expr->ref->next != NULL
>> +  && n->expr->ref->next->type == REF_ARRAY
>> +  && n->expr->ref->next->u.ar.type == AR_FULL)))
>> +{
>> +  gfc_ref *ref = n->expr->ref;
>> +  tree orig_decl = decl;
> 
> [...]/source-gcc/gcc/fortran/trans-openmp.c: In function 'tree_node* 
> gfc_trans_omp_clauses_1(stmtblock_t*, gfc_omp_clauses*, locus, bool)':
> [...]/source-gcc/gcc/fortran/trans-openmp.c:1947:10: warning: unused 
> variable 'orig_decl' [-Wunused-variable]
>  tree orig_decl = decl;
>   ^

Not sure why this wasn't caught with a bootstrap build. Anyway, I've the
attached patch to gomp4 to fix this issue. It also corrects a problem
that you found with checking enabled.

>> --- /dev/null
>> +++ b/gcc/testsuite/gfortran.dg/goacc/derived-types.f90
>> @@ -0,0 +1,78 @@
>> +! Test ACC UPDATE with derived types. The DEVICE clause depends on an
>> +! accelerator being present.
> 
> I guess that "DEVICE" comment here is a leftover?  (Doesn't apply to a
> compile test.)
> 
>> +module dt
>> +  integer, parameter :: n = 10
>> +  type inner
>> + integer :: d(n)
>> +  end type inner
>> +  type dtype
>> + integer(8) :: a, b, c(n)
>> + type(inner) :: in
>> +  end type dtype
>> +end module dt
>> +
>> +program derived_acc
>> +  use dt
>> +  
>> +  implicit none
>> +  type(dtype):: var
>> +  integer i
>> +  !$acc declare create(var)
>> +  !$acc declare pcopy(var%a) ! { dg-error "Syntax error in OpenMP" }
>> +
>> +  !$acc update host(var)
>> +  !$acc update host(var%a)
>> +  !$acc update device(var)
>> +  !$acc update device(var%a)
>> +[...]
> 
>> --- /dev/null
>> +++ b/libgomp/testsuite/libgomp.oacc-fortran/update-2.f90
>> @@ -0,0 +1,285 @@
>> +! Test ACC UPDATE with derived types. The DEVICE clause depends on an
>> +! accelerator being present.
> 
> Why?  Shouldn "!$acc update device" just be a no-op for host execution?

Just more test coverage.

Cesar

2017-02-01  Cesar Philippidis  

	gcc/fortran/
	* openmp.c (gfc_match_omp_variable_list): Eliminate a warning when
	checking for derived types.
	* trans-openmp.c (gfc_trans_omp_clauses_1): Don't cast derived type
	pointers to void pointers.


diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 2782a8d..b3506d4 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -243,8 +243,8 @@ gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
 	case MATCH_YES:
 	  gfc_expr *expr;
 	  expr = NULL;
-	  if (allow_sections && gfc_peek_ascii_char () == '('
-	  || allow_derived && gfc_peek_ascii_char () == '%')
+	  if ((allow_sections && gfc_peek_ascii_char () == '(')
+	  || (allow_derived && gfc_peek_ascii_char () == '%'))
 	{
 	  gfc_current_locus = cur_loc;
 	  m = gfc_match_variable (, 0);
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 7826e1c..80aa421 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -1986,9 +1986,10 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, gfc_omp_clauses *clauses,
 	 TREE_TYPE (field), decl, field,
 	 NULL_TREE);
 		  type = TREE_TYPE (scratch);
-		  ptr =

Re: [gomp4] add support for derived types in ACC UPDATE

2017-01-30 Thread Thomas Schwinge

Hi Cesar!

On Thu, 10 Nov 2016 09:38:33 -0800, Cesar Philippidis  
wrote:
> This patch has been committed to gomp-4_0-branch.

> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c

> @@ -242,7 +243,8 @@ gfc_match_omp_variable_list (const char *str, 
> gfc_omp_namelist **list,
>   case MATCH_YES:
> gfc_expr *expr;
> expr = NULL;
> -   if (allow_sections && gfc_peek_ascii_char () == '(')
> +   if (allow_sections && gfc_peek_ascii_char () == '('
> +   || allow_derived && gfc_peek_ascii_char () == '%')
>   {
> gfc_current_locus = cur_loc;
> m = gfc_match_variable (, 0);

[...]/source-gcc/gcc/fortran/openmp.c: In function 'match 
{+gfc_match_omp_variable_list(const char*, gfc_omp_namelist**, bool, bool*, 
gfc_omp_namelist***, bool, bool)':+}
[...]/source-gcc/gcc/fortran/openmp.c:246:23: warning: suggest parentheses 
around '&&' within '||' [-Wparentheses]
if (allow_sections && gfc_peek_ascii_char () == '('
   ^

> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -1938,7 +1938,66 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, 
> gfc_omp_clauses *clauses,
> tree decl = gfc_get_symbol_decl (n->sym);
> if (DECL_P (decl))
>   TREE_ADDRESSABLE (decl) = 1;
> -   if (n->expr == NULL || n->expr->ref->u.ar.type == AR_FULL)
> +   /* Handle derived-typed members for OpenACC Update.  */
> +   if (n->sym->ts.type == BT_DERIVED
> +   && n->expr != NULL && n->expr->ref != NULL
> +   && (n->expr->ref->next == NULL
> +   || (n->expr->ref->next != NULL
> +   && n->expr->ref->next->type == REF_ARRAY
> +   && n->expr->ref->next->u.ar.type == AR_FULL)))
> + {
> +   gfc_ref *ref = n->expr->ref;
> +   tree orig_decl = decl;

[...]/source-gcc/gcc/fortran/trans-openmp.c: In function 'tree_node* 
gfc_trans_omp_clauses_1(stmtblock_t*, gfc_omp_clauses*, locus, bool)':
[...]/source-gcc/gcc/fortran/trans-openmp.c:1947:10: warning: unused 
variable 'orig_decl' [-Wunused-variable]
 tree orig_decl = decl;
  ^

> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/goacc/derived-types.f90
> @@ -0,0 +1,78 @@
> +! Test ACC UPDATE with derived types. The DEVICE clause depends on an
> +! accelerator being present.

I guess that "DEVICE" comment here is a leftover?  (Doesn't apply to a
compile test.)

> +module dt
> +  integer, parameter :: n = 10
> +  type inner
> + integer :: d(n)
> +  end type inner
> +  type dtype
> + integer(8) :: a, b, c(n)
> + type(inner) :: in
> +  end type dtype
> +end module dt
> +
> +program derived_acc
> +  use dt
> +  
> +  implicit none
> +  type(dtype):: var
> +  integer i
> +  !$acc declare create(var)
> +  !$acc declare pcopy(var%a) ! { dg-error "Syntax error in OpenMP" }
> +
> +  !$acc update host(var)
> +  !$acc update host(var%a)
> +  !$acc update device(var)
> +  !$acc update device(var%a)
> +[...]

> --- /dev/null
> +++ b/libgomp/testsuite/libgomp.oacc-fortran/update-2.f90
> @@ -0,0 +1,285 @@
> +! Test ACC UPDATE with derived types. The DEVICE clause depends on an
> +! accelerator being present.

Why?  Shouldn "!$acc update device" just be a no-op for host execution?


Grüße
 Thomas


> +! { dg-do run  { target openacc_nvidia_accel_selected } }
> +
> +module dt
> +  integer, parameter :: n = 10
> +  type inner
> + integer :: d(n)
> +  end type inner
> +  type mytype
> + integer(8) :: a, b, c(n)
> + type(inner) :: in
> +  end type mytype
> +end module dt
> +
> +program derived_acc
> +  use dt
> +
> +  implicit none
> +  integer i, res
> +  type(mytype) :: var
> +
> +  var%a = 0
> +  var%b = 1
> +  var%c(:) = 10
> +  var%in%d(:) = 100
> +
> +  var%c(:) = 10
> +
> +  !$acc enter data copyin(var)
> +
> +  !$acc parallel loop present(var)
> +  do i = 1, 1
> + var%a = var%b
> +  end do
> +  !$acc end parallel loop
> +
> +  !$acc update host(var%a)
> +
> +  if (var%a /= var%b) call abort
> +
> +  var%b = 100
> +
> +  !$acc update device(var%b)
> +
> +  !$acc parallel loop present(var)
> +  do i = 1, 1
> + var%a = var%b
> +  end do
> +  !$acc end parallel loop
> +
> +  !$acc update host(var%a)
> +
> +  if (var%a /= var%b) call abort
> +
> +  !$acc parallel loop present (var)
> +  do i = 1, n
> + var%c(i) = i
> +  end do
> +  !$acc end parallel loop
> +
> +  !$acc update host(var%c)
> +
> +  var%a = -1
> +
> +  do i = 1, n
> + if (var%c(i) /= i) call abort
> + var%c(i) = var%a
> +  end do
> +
> +  !$acc update device(var%a)
> +  !$acc update device(var%c)
> +
> +  res = 0
> +
> +  !$acc parallel loop present(var) reduction(+:res)
> +  do i = 1, n
> + if (var%c(i) /= var%a) res = res + 1
> +  end do
> +
> +  if (res /= 0) call abort
> +
> +  var%c(:) = 0
> +
> +  !$acc update device(var%c)
> +
> +  !$acc parallel loop

[gomp4] add support for derived types in ACC UPDATE

2016-11-10 Thread Cesar Philippidis

OpenACC 2.0a has limited support for fortran derived types. Basically,
derived type variables are only supported in ACC UPDATE. Rather than
adding generalized support for derived times in the gimplifier, this
patch has the fortran FE pass both subarrays and arrays as void pointers
with an appropriate OMP_CLAUSE_SIZE. ACC UPDATE is an executable
directive, and the gimplifier would just end up pruning out all of the
unnecessary supporting data clauses otherwise.

As of right now, GCC still lacks support for non-contiguous subarray
arguments to ACC UPDATE. I'm not sure why it was decided to let ACC
UPDATE support non-contiguous subarrays, but it already is an oddball
with its lone support for derived types.

This patch has been committed to gomp-4_0-branch.

Cesar
2016-11-10  Cesar Philippidis  

	gcc/fortran/
	* openmp.c (gfc_match_omp_variable_list): New allow_derived argument.
	(gfc_match_omp_map_clause): Update call to
	gfc_match_omp_variable_list.
	(gfc_match_omp_clauses): Update calls to gfc_match_omp_map_clause.
	(gfc_match_oacc_update): Update call to gfc_match_omp_clauses.
	(resolve_omp_clauses): Permit derived type variables in ACC UPDATE
	clauses.
	* trans-openmp.c (gfc_trans_omp_clauses_1): Lower derived type
	members.

	gcc/
	* gimplify.c (gimplify_scan_omp_clauses): Update handling of ACC
	UPDATE variables.

	gcc/testsuite/
	* gfortran.dg/goacc/derived-types.f90: New test.

	libgomp/
	* testsuite/libgomp.oacc-fortran/update-2.f90: New test.


diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 95885bc..0a9d137 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -216,7 +216,8 @@ static match
 gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
 			 bool allow_common, bool *end_colon = NULL,
 			 gfc_omp_namelist ***headp = NULL,
-			 bool allow_sections = false)
+			 bool allow_sections = false,
+			 bool allow_derived = false)
 {
   gfc_omp_namelist *head, *tail, *p;
   locus old_loc, cur_loc;
@@ -242,7 +243,8 @@ gfc_match_omp_variable_list (const char *str, gfc_omp_namelist **list,
 	case MATCH_YES:
 	  gfc_expr *expr;
 	  expr = NULL;
-	  if (allow_sections && gfc_peek_ascii_char () == '(')
+	  if (allow_sections && gfc_peek_ascii_char () == '('
+	  || allow_derived && gfc_peek_ascii_char () == '%')
 	{
 	  gfc_current_locus = cur_loc;
 	  m = gfc_match_variable (, 0);
@@ -634,10 +636,11 @@ cleanup:
 
 static bool
 gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op,
-			  bool common_blocks)
+			  bool common_blocks, bool allow_derived)
 {
   gfc_omp_namelist **head = NULL;
-  if (gfc_match_omp_variable_list ("", list, common_blocks, NULL, , true)
+  if (gfc_match_omp_variable_list ("", list, common_blocks, NULL, , true,
+   allow_derived)
   == MATCH_YES)
 {
   gfc_omp_namelist *n;
@@ -655,7 +658,8 @@ gfc_match_omp_map_clause (gfc_omp_namelist **list, gfc_omp_map_op map_op,
 static match
 gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 		   uint64_t dtype_mask, bool first = true,
-		   bool needs_space = true, bool openacc = false)
+		   bool needs_space = true, bool openacc = false,
+		   bool allow_derived = false)
 {
   gfc_omp_clauses *base_clauses, *c = gfc_get_omp_clauses ();
   locus old_loc;
@@ -773,7 +777,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 	  if ((mask & OMP_CLAUSE_COPY)
 	  && gfc_match ("copy ( ") == MATCH_YES
 	  && gfc_match_omp_map_clause (>lists[OMP_LIST_MAP],
-	   OMP_MAP_FORCE_TOFROM, openacc))
+	   OMP_MAP_FORCE_TOFROM, openacc,
+	   allow_derived))
 	continue;
 	  if (mask & OMP_CLAUSE_COPYIN)
 	{
@@ -781,7 +786,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 		{
 		  if (gfc_match ("copyin ( ") == MATCH_YES
 		  && gfc_match_omp_map_clause (>lists[OMP_LIST_MAP],
-		   OMP_MAP_FORCE_TO, true))
+		   OMP_MAP_FORCE_TO, true,
+		   allow_derived))
 		continue;
 		}
 	  else if (gfc_match_omp_variable_list ("copyin (",
@@ -792,7 +798,8 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 	  if ((mask & OMP_CLAUSE_COPYOUT)
 	  && gfc_match ("copyout ( ") == MATCH_YES
 	  && gfc_match_omp_map_clause (>lists[OMP_LIST_MAP],
-	   OMP_MAP_FORCE_FROM, true))
+	   OMP_MAP_FORCE_FROM, true,
+	   allow_derived))
 	continue;
 	  if ((mask & OMP_CLAUSE_COPYPRIVATE)
 	  && gfc_match_omp_variable_list ("copyprivate (",
@@ -802,14 +809,15 @@ gfc_match_omp_clauses (gfc_omp_clauses **cp, uint64_t mask,
 	  if ((mask & OMP_CLAUSE_CREATE)
 	  && gfc_match ("create ( ") == MATCH_YES
 	  && gfc_match_omp_map_clause (>lists[OMP_LIST_MAP],
-	   OMP_MAP_FORCE_ALLOC, true))
+	   OMP_MAP_FORCE_ALLOC, true,
+	   allow_derived))
 	continue;
 	  break;
 	case 'd':
 	  if ((mask & OMP_CLAUSE_DELETE)
 	  && gfc_match ("delete ( ") == MATCH_YES
 	  &&

Re: [gomp4] add support for derived types in ACC UPDATE

Re: [gomp4] add support for derived types in ACC UPDATE

[gomp4] add support for derived types in ACC UPDATE

3 matches

Site Navigation

Mail list logo

Footer information