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

[Cesar Philippidis <cesar at codesourcery dot com>]

On 01/30/2017 02:08 AM, Thomas Schwinge wrote:
> Hi Cesar!
> 
> On Thu, 10 Nov 2016 09:38:33 -0800, Cesar Philippidis <cesar@codesourcery.com> 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 (&expr, 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  <cesar@codesourcery.com>

	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 (&expr, 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 = gfc_create_var (build_pointer_type (void_type_node),
-					NULL);
-		  gfc_add_modify (block, ptr, build_fold_addr_expr (scratch));
+		  ptr = gfc_create_var (pvoid_type_node, NULL);
+		  scratch = fold_convert (pvoid_type_node,
+					  build_fold_addr_expr (scratch));
+		  gfc_add_modify (block, ptr, scratch);
 		  OMP_CLAUSE_SIZE (node) = TYPE_SIZE_UNIT (type);
 		  OMP_CLAUSE_DECL (node) = build_fold_indirect_ref (ptr);
 		}
@@ -2111,6 +2112,7 @@ gfc_trans_omp_clauses_1 (stmtblock_t *block, gfc_omp_clauses *clauses,
 			  tree t = gfc_create_var (build_pointer_type
 						   (void_type_node),
 						   NULL);
+			  ptr = fold_convert (pvoid_type_node, ptr);
 			  gfc_add_modify (block, t, ptr);
 			  ptr = t;
 			  type = TREE_TYPE (type);