Bug 92036 - OpenACC 'firstprivate' clause: initial value
Summary: OpenACC 'firstprivate' clause: initial value
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: unknown
: P3 normal
Target Milestone: ---
Assignee: jules
URL:
Keywords: openacc
Depends on:
Blocks:
 
Reported: 2019-10-09 11:27 UTC by Thomas Schwinge
Modified: 2019-10-09 11:45 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-10-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schwinge 2019-10-09 11:27:19 UTC
I think our implementation of OpenACC 'firstprivate' clause (on the OpenACC 'parallel' construct) is wrong regarding where such a variable gets initialized from if a device copy already exists.

What we currently seem to be doing, in case that the respective original variable already has a device copy, is to initialize the 'firstprivate' copy from the *device* copy, which potentially might have a different value.  However, the OpenACC 2.0a specification in 2.5.10 "firstprivate clause" asks that "the copy will be initialized with the value of that item *on the host* when the 'parallel' construct is encountered" (emphasis mine), in other words (my interpretation), initialized by the dynamic (current) value of the *original* variable (specified in the 'firstprivate' clause) at the time the OpenACC 'parallel' construct is encountered.

The current OpenACC 2.7 specification in 2.5.12. "firstprivate clause" asks that "the copy will be initialized with the value of that item *on the local thread* when a 'parallel' or 'serial' construct is encountered" (emphasis mine).  The term "local thread" in 6. "Glossary" gets defined as "the host thread [...] that executes an OpenACC directive or construct" (omitted the variant "or the accelerator thread" as that's (a) still the same semantics, and (b) not relevant as long as we're not implementing OpenACC nested parallelism).  So, I read that as the same as the original OpenACC 2.0a.

This, per my current understanding, matches the semantics of the OpenMP 'firstprivate' clause.  (Jakub?)

I suppose this needs to be re-worked (at least) in the gimplifier?

Not sure if that's related, but as far as I understand, the OpenACC handling of 'OMP_CLAUSE_FIRSTPRIVATE' in 'gcc/omp-low.c' is rather different from the OpenMP handling -- might that also be relevant to this issue here?
Comment 1 Thomas Schwinge 2019-10-09 11:31:45 UTC
Author: tschwinge
Date: Wed Oct  9 11:31:14 2019
New Revision: 276757

URL: https://gcc.gnu.org/viewcvs?rev=276757&root=gcc&view=rev
Log:
[PR92036] Add 'libgomp.oacc-c-c++-common/data-firstprivate-1.c'

	libgomp/
	PR middle-end/92036
	* testsuite/libgomp.oacc-c-c++-common/data-firstprivate-1.c: New
	file.

Added:
    trunk/libgomp/testsuite/libgomp.oacc-c-c++-common/data-firstprivate-1.c
Modified:
    trunk/libgomp/ChangeLog
Comment 2 Thomas Schwinge 2019-10-09 11:39:14 UTC
Julian, please have a look: intention of the OpenACC specification vs. current GCC implementation.  Probably a good idea to also cross-verify with another OpenACC compiler.


(Why this is relevant right now: in openacc-gcc-9-branch, we have a few patches related to 'firstprivate', but I think before considering these for GCC trunk, we'll need to resolve (or reject, if invalid) this issue here.)
Comment 3 Thomas Schwinge 2019-10-09 11:45:15 UTC
If I quickly got that right, the current implementation is described in the thread starting at <http://mid.mail-archive.com/563E01A4.20607@acm.org> -- and Nathan seems to have had a different idea of what the expected semantics are, contrary to what I just described.  But in turn, my recent description seems to match that Jakub expected back then.


Please do not yet work on any 'GOMP_MAP_FIRSTPRIVATE_INT' things, unless what naturally falls out when (possibly) revising the implementation.  (Related to that, I have some further pending changes related to our openacc-gcc-9-branch commits for 'GOMP_MAP_FIRSTPRIVATE_INT', which is how I found this issue.)