Re: [gomp4 8/8] libgomp: provide ICVs via env.c on nvptx

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Sep 24, 2015 at 04:15:28PM +0300, Alexander Monakov wrote:
> Definitely, thanks for the suggestion!  While implementing that, I considered
> that it should be more natural to keep only env processing in env.c, and split
> device-related functionality in another file, icv-device.c.  That way, nvptx
> can keep a zero-sized env.c, use generic icv.c, and provide its overrides in
> icv-device.c.  If that's too fancy I can revert to your suggested approach.
> How does the following patch look?

icv-device.c looks reasonable.  Note, the wording is that it is UB if (some
of those) functions are called from target regions.  That means the routines
still should be defined somewhere, but can be just stubbed.

> [gomp4] libgomp: split ICV functionality out of env.c
> 
> Split env.c, leaving only processing of environment variables in the original
> file.  Move most of ICV definitions and associated API entrypoints into icv.c,
> except target-related API entrypoints, which are moved into icv-device.c.  The
> intention is to allow offload-only architectures to use the generic icv.c.
> 
> 	* Makefile.am (libgomp_la_SOURCES): Add icv.c and icv-device.c.
>         * Makefile.in: Regenerate.
>         * env.c: Split out ICV definitions into...
>         * icv.c: ...here (new file) and...
>         * icv-device.c: ...here. New file.

LGTM, except:

> @@ -95,6 +95,10 @@ fortran.lo: libgomp_f.h
>  fortran.o: libgomp_f.h
>  env.lo: libgomp_f.h
>  env.o: libgomp_f.h
> +icv.lo: libgomp_f.h
> +icv.o: libgomp_f.h
> +icv-device.lo: libgomp_f.h
> +icv-device.o: libgomp_f.h

You don't really want this, it is enough to include it in env.c only.

> +/* This file defines OpenMP API entry points that accelerator targets are
> +   expected to replace.  */
> +
> +#include "libgomp.h"
> +#include "libgomp_f.h"

And please leave out the libgomp_f.h include here.

> --- /dev/null
> +++ b/libgomp/icv.c
> @@ -0,0 +1,181 @@
> +/* Copyright (C) 2015 Free Software Foundation, Inc.

I'd say as the file is a copy of the source of env.c that originates back to
2005, it should be 2005-2015 (both files).

> +/* This file defines the OpenMP internal control variables and associated
> +   OpenMP API entry points.  */
> +
> +#include "libgomp.h"
> +#include "libgomp_f.h"

Like above.

Just to make sure, ChangeLog entries on the gomp-4*-branch branches go into
ChangeLog.gomp.

	Jakub