Re: [patch] Update support for Fortran arrays in OpenACC

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Jun 29, 2018 at 11:07:48AM -0700, Cesar Philippidis wrote:
> On 06/29/2018 10:49 AM, Jakub Jelinek wrote:
> > On Fri, Jun 29, 2018 at 10:33:56AM -0700, Cesar Philippidis wrote:
> >> @@ -1044,21 +1046,6 @@ gfc_omp_finish_clause (tree c, gimple_seq *pre_p)
> >>      return;
> >>  
> >>    tree decl = OMP_CLAUSE_DECL (c);
> >> -
> >> -  /* Assumed-size arrays can't be mapped implicitly, they have to be
> >> -     mapped explicitly using array sections.  */
> >> -  if (TREE_CODE (decl) == PARM_DECL
> >> -      && GFC_ARRAY_TYPE_P (TREE_TYPE (decl))
> >> -      && GFC_TYPE_ARRAY_AKIND (TREE_TYPE (decl)) == GFC_ARRAY_UNKNOWN
> >> -      && GFC_TYPE_ARRAY_UBOUND (TREE_TYPE (decl),
> >> -				GFC_TYPE_ARRAY_RANK (TREE_TYPE (decl)) - 1)
> >> -	 == NULL)
> >> -    {
> >> -      error_at (OMP_CLAUSE_LOCATION (c),
> >> -		"implicit mapping of assumed size array %qD", decl);
> >> -      return;
> >> -    }
> >> -
> >>    tree c2 = NULL_TREE, c3 = NULL_TREE, c4 = NULL_TREE;
> >>    if (POINTER_TYPE_P (TREE_TYPE (decl)))
> >>      {
> > 
> > I don't have time to review this fully right now, but the above looks like a
> > blocker to me.  The above must be diagnosed for OpenMP, so taking it away
> > rather than say conditionalizing it on whether it is in an OpenMP or OpenACC
> > construct is just wrong.
> 
> In certain respects, the above code is overly strict if the data is
> already present on the device. However, I do see your point. Would you
> be OK if I reduced that error to a warning?

No, it is violating the standard requirements for OpenMP, so it should be
an error.

> > As a general feeling of the patch there are many other spots that change
> > unconditionally code used by OpenMP and OpenACC and it isn't clear it
> > doesn't affect OpenMP code generation.  If some change is useful even for
> > OpenMP and Fortran, then I'd certainly expect it to be done only in omp-low
> > or omp-expand, before that it better should be represented how the standard
> > mandates.
> 
> I'll add more comments to the code. Also, I admit that I should make a
> stronger effort to share code between OpenACC and OpenMP. Would you be
> interested in using GOMP_MAP_FIRSTPRIVATE_POINTER mappings for arrays in
> OpenMP? I'm not sure if that's supported by OpenMP, although even with
> OpenACC it's not used everywhere yet.

GOMP_MAP_FIRSTPRIVATE_POINTER is (at least for OpenMP) standard mandated
behavior, which is for C/C++ pointers only, not for Fortran arrays.

	Jakub