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
