On Fri, 5 Feb 2021 17:25:10 +0100
Tobias Burnus <tobias@codesourcery.com> wrote:

> (CC fortran@)
> 
> Hi Julian,
> 
> not doing an extensive review yet, but the following gives an ICE
> with this patch applied. (I believe the others are already in, aren't
> they?)
> 
> type t
>   integer :: i, j
> end type t
> type t2
>   type(t) :: b(4)
> end type
> type(t2) :: var(10)
> !$acc update host(var(3)%b(:)%j)
> !$acc update host(var(3)%b%j)
> end
> 
> That's a noncontiguous array – which is permitted for 'update'
> and it gives an ICE via:
> 
> 0x9b0c59 gfc_conv_scalarized_array_ref
>          ../../repos/gcc/gcc/fortran/trans-array.c:3570
> 0x9b2134 gfc_conv_array_ref(gfc_se*, gfc_array_ref*, gfc_expr*,
> locus*) ../../repos/gcc/gcc/fortran/trans-array.c:3721
> 0x9e9cc6 gfc_conv_variable
>          ../../repos/gcc/gcc/fortran/trans-expr.c:2998
> 0xa22682 gfc_trans_omp_clauses
>          ../../repos/gcc/gcc/fortran/trans-openmp.c:2963

I think the attached patch fixes that. (This could be merged into the
parent patch or kept separate, not sure which is better.)

Re-tested with offloading to AMD GCN. OK?

> > +	      bool allocatable = false, pointer = false;
> > +
> > +	      if (lastref && lastref->type == REF_COMPONENT)
> > +		{
> > +		  gfc_component *c = lastref->u.c.component;
> > +
> > +		  if (c->ts.type == BT_CLASS)
> > +		    {
> > +		      pointer = CLASS_DATA (c)->attr.class_pointer;
> > +		      allocatable = CLASS_DATA
> > (c)->attr.allocatable;
> > +		    }
> > +		  else
> > +		    {
> > +		      pointer = c->attr.pointer;
> > +		      allocatable = c->attr.allocatable;
> > +		    }
> > +		}
> > +  
> 
> I am not sure how the rest will change, but I was wondering
> whether the following helps. I see that 'lastref' is used
> elsewhere – hence, I am not sure whether it is indeed better.
>   
> symbol_attribute attr = {};
> if (n->expr)
>    attr = gfc_expr_attr (n->expr);

Ah, I didn't know about that one! But yeah, not sure if it's better
here.

Thanks for (pre-)review!

Julian