Re: [gomp4.1] map clause parsing improvements

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Oct 19, 2015 at 12:20:23PM +0200, Thomas Schwinge wrote:
> > @@ -77,7 +79,21 @@ enum gomp_map_kind
> >      /* ..., and copy from device.  */
> >      GOMP_MAP_FORCE_FROM =		(GOMP_MAP_FLAG_FORCE | GOMP_MAP_FROM),
> >      /* ..., and copy to and from device.  */
> > -    GOMP_MAP_FORCE_TOFROM =		(GOMP_MAP_FLAG_FORCE | GOMP_MAP_TOFROM)
> > +    GOMP_MAP_FORCE_TOFROM =		(GOMP_MAP_FLAG_FORCE | GOMP_MAP_TOFROM),
> > +    /* If not already present, allocate.  And unconditionally copy to
> > +       device.  */
> > +    GOMP_MAP_ALWAYS_TO =		(GOMP_MAP_FLAG_ALWAYS | GOMP_MAP_TO),
> > +    /* If not already present, allocate.  And unconditionally copy from
> > +       device.  */
> > +    GOMP_MAP_ALWAYS_FROM =		(GOMP_MAP_FLAG_ALWAYS | GOMP_MAP_FROM),
> > +    /* If not already present, allocate.  And unconditionally copy to and from
> > +       device.  */
> > +    GOMP_MAP_ALWAYS_TOFROM =		(GOMP_MAP_FLAG_ALWAYS | GOMP_MAP_TOFROM),
> > +    /* OpenMP 4.1 alias for forced deallocation.  */
> > +    GOMP_MAP_DELETE =			GOMP_MAP_FORCE_DEALLOC,
> 
> To avoid confusion about two different identifiers naming the same
> functionality, I'd prefer to avoid such aliases ("GOMP_MAP_DELETE =
> GOMP_MAP_FORCE_DEALLOC"), and instead just rename GOMP_MAP_FORCE_DEALLOC
> to GOMP_MAP_DELETE, if that's the name you prefer.

If you are ok with removing GOMP_MAP_FORCE_DEALLOC and just use
GOMP_MAP_DELETE, that is ok by me, just post a patch.

> By the way, looking at GCC 6 libgomp compatibility regarding
> OpenACC/nvptx offloading for executables compiled with GCC 5, for the
> legacy entry point libgomp/oacc-parallel.c:GOACC_parallel only supports
> host-fallback execution, which doesn't pay attention to data clause at
> all (sizes and kinds formal parameters), so you're free to renumber
> GOMP_MAP_* if/where that makes sense.
> 
> > +    /* Decrement usage count and deallocate if zero.  */
> > +    GOMP_MAP_RELEASE =			(GOMP_MAP_FLAG_ALWAYS
> > +					 | GOMP_MAP_FORCE_DEALLOC)
> >    };
> 
> I have not yet read the OpenMP 4.1/4.5 standard, but it's not obvious to
> me here how the GOMP_MAP_FLAG_ALWAYS flag relates to the OpenMP release
> clause (GOMP_MAP_RELEASE here)?  Shouldn't GOMP_MAP_RELEASE be
> "(GOMP_MAP_FLAG_SPECIAL_1 | 3)" or similar?

It isn't related to always, but always really is something that affects
solely the data movement (i.e. to, from, tofrom), and while it can be
specified elsewhere, it makes no difference.  Wasting one bit just for that
is something we don't have the luxury for, which is why I've started using
that bit for other OpenMP stuff (it acts there like GOMP_MAP_FLAG_SPECIAL_2
to some extent).  It is not just release, but also the struct mapping etc.
I'll still need to make further changes, because the rules for mapping
structure element pointer/reference based array sections and structure
element references have changed again.

Some changes in the enum can be of course still be done until say mid stage3
but at least for OpenMP 4.0 we should keep backwards compatibility (so
whatever we've already used in GCC 4.9/5 should keep working).

	Jakub