[PATCH 3/3] OpenACC 2.6 manual deep copy support (attach/detach)

[Bernhard Reutner-Fischer]

On Sat, 10 Nov 2018 09:11:20 -0800
Julian Brown <julian@codesourcery.com> wrote:

Two nits.

> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
> index 6430e61..ebba7ca 100644
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c

> @@ -3781,9 +3805,6 @@ check_array_not_assumed (gfc_symbol *sym, locus loc, const char *name)
>  static void
>  resolve_oacc_data_clauses (gfc_symbol *sym, locus loc, const char *name)
>  {
> -  if (sym->ts.type == BT_DERIVED && sym->attr.allocatable)
> -    gfc_error ("ALLOCATABLE object %qs of derived type in %s clause at %L",
> -	       sym->name, name, &loc);
>    if ((sym->ts.type == BT_ASSUMED && sym->attr.allocatable)
>        || (sym->ts.type == BT_CLASS && CLASS_DATA (sym)
>  	  && CLASS_DATA (sym)->attr.allocatable))
> @@ -4153,11 +4174,23 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses,
>  	&& (list != OMP_LIST_REDUCTION || !openacc))
>        for (n = omp_clauses->lists[list]; n; n = n->next)
>  	{
> -	  if (n->sym->mark)
> -	    gfc_error ("Symbol %qs present on multiple clauses at %L",
> -		       n->sym->name, &n->where);
> -	  else
> -	    n->sym->mark = 1;
> +	  bool array_only_p = true;
> +	  /* Disallow duplicate bare variable references and multiple
> +	     subarrays of the same array here, but allow multiple components of
> +	     the same (e.g. derived-type) variable.  For the latter, duplicate
> +	     components are detected elsewhere.  */
> +	  if (openacc && n->expr && n->expr->expr_type == EXPR_VARIABLE)
> +	    for (gfc_ref *ref = n->expr->ref; ref; ref = ref->next)
> +	      if (ref->type != REF_ARRAY)
> +		array_only_p = false;

you could break here.
It seems we do not perform this optimization ourself even when we
could (or should) prove that the dataflow does not force the loop to run
to completion?!

Consider:

int foo(char *str) {                                                            
        _Bool cond = 0;                                                         
        for (char *chp = str; *chp != 0; chp++)                                 
                if (*chp == 42) {                                               
                        cond = 1;                                               
#ifdef BREAK                                                                    
                        break;                                                  
#endif                                                                          
                }                                                               
        return 0x0815 + cond;                                                   
}

$ for i in 0 1 2 3 s;do gcc -UBREAK -O$i -c -o $i.o /tmp/1/foo.c  -fomit-frame-pointer -m32 -mtune=i386;done;size ?.o
   text	   data	    bss	    dec	    hex	filename
    109	      0	      0	    109	     6d	0.o
     97	      0	      0	     97	     61	1.o
    107	      0	      0	    107	     6b	2.o
    107	      0	      0	    107	     6b	3.o
     86	      0	      0	     86      56	s.o
$ for i in 0 1 2 3 s;do gcc -DBREAK -O$i -c -o $i.o /tmp/1/foo.c  -fomit-frame-pointer -m32 -mtune=i386;done;size ?.o
   text	   data	    bss	    dec	    hex	filename
    111	      0	      0	    111	     6f	0.o
     82	      0	      0	     82	     52	1.o
     82	      0	      0	     82	     52	2.o
     82	      0	      0	     82	     52	3.o
     77	      0	      0	     77	     4d	s.o

respectively

$ for i in 0 1 2 3 s;do gcc -UBREAK -O$i -c -o $i.o /tmp/1/foo.c  -fomit-frame-pointer -m32 -mtune=i386 -falign-functions=1 -falign-jumps=1 -falign-labels=1 -falign-loops=1 -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-guess-branch-probability;done;size ?.o
   text	   data	    bss	    dec	    hex	filename
     61	      0	      0	     61	     3d	0.o
     38	      0	      0	     38	     26	1.o
     43	      0	      0	     43	     2b	2.o
     43	      0	      0	     43	     2b	3.o
     34	      0	      0	     34	     22	s.o
$ for i in 0 1 2 3 s;do gcc -DBREAK -O$i -c -o $i.o /tmp/1/foo.c  -fomit-frame-pointer -m32 -mtune=i386 -falign-functions=1 -falign-jumps=1 -falign-labels=1 -falign-loops=1 -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-guess-branch-probability;done;size ?.o
   text	   data	    bss	    dec	    hex	filename
     63	      0	      0	     63	     3f	0.o
     39	      0	      0	     39	     27	1.o
     39	      0	      0	     39	     27	2.o
     39	      0	      0	     39	     27	3.o
     33	      0	      0	     33	     21	s.o

that's really a pity.


> +	  if (array_only_p)
> +	    {
> +	      if (n->sym->mark)
> +		gfc_error ("Symbol %qs present on multiple clauses at %L",
> +			   n->sym->name, &n->where);
> +	      else
> +		n->sym->mark = 1;
> +	    }
>  	}

> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 274edc0..aa7723d 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c

> +		      /* Turning a GOMP_MAP_ALWAYS_POINTER clause into a
> +			 GOMP_MAP_ATTACH clause after we have detected a case
> +			 that needs a GOMP_MAP_STRUCT mapping adding.  */

s/adding/added/ i think.

thanks,