Hi PA,
regarding this patch: There is now a minor conflict in gomp_update
due to the the support of the GOMP_RUNTIME_USM=enabled support.
I think just having the
if ((devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
&& n->refcount != REFCOUNT_INFINITY)
continue;
after 'if (n)' in both the if GRID and the else branch is enough.
(It is there already in the 'else' part by by adding 'else' it
needs to be re-indented.)
That features permits to enable unified-shared memory by turning
everything in self maps, but that won't work for global variables
like:
int array[100*100];
#pragma omp declare target enter(array)
as they are still on the device - such that
#pragma omp declare update to( array[...] )
is honored only for device-resident variables when the system
supports unified shared memory and GOMP_RUNTIME_USM=enabled is
visible to the runtime (does not happen for remote DejagGNU testing).
I think a small testcase for this would be nice,
cf. libgomp/testsuite/libgomp.c/usm_env_handling-+.c for some
examples.
I guess for the pointer-attach strided patch, it will also need some
tweaks. (BTW: the GOMP_RUNTIME_USM patch is about to land on OG16.)
* * *
I just realized that I missed to reply to the following email ...
Paul-Antoine Arras wrote:
When I just removed the AFFINITY/DEPEND exclusion, I ran into an ICE.
That was because there was a call site where discontiguous was set to
true specifically for these two clauses. I set it to false and did not
see any regression.
Ups, did I miss a call site? – In any case, as far as I can see, only TO and
FROM
permit strides:
Namely:
* An array section can appear only in clauses for which it is explicitly
allowed.7
* A stride expression may not be specified unless otherwise stated.
And while array sections are permitted for affinity + depend, stride is not
mentioned
(and one stray 'stride' mentioned in a restriction I removed in the OpenMP 6.1
QC
pass - it was pointless as not allowed in general). - And for C/C++, I also
fail to
see how it can be non-contiguous when there is no stride.
Thus the following change makes completely sense, except that the comment
in handle_omp_array_sections_1 can now be removed as well:
-- gcc/cp/semantics.cc
+++ gcc/cp/semantics.cc
@@ -6530,9 +6530,7 @@ handle_omp_array_sections_1 (tree c, tree t,
vec<tree> &types,
/* AFFINITY/DEPEND only need an address, not real array-section
semantics;
stride/length are discarded for them here even when a discontiguous
section is permitted. */
- if (*discontiguous
- && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_AFFINITY
- && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND)
+ if (*discontiguous)
ret = grok_omp_array_section (OMP_CLAUSE_LOCATION (c), ret,
low_bound,
length, stride);
else
@@ -9412,7 +9410,7 @@ finish_omp_clauses (tree clauses, enum
c_omp_region_type ort)
if (TREE_CODE (t) == OMP_ARRAY_SECTION)
{
- bool discontiguous = true;
+ bool discontiguous = false;
if (handle_omp_array_sections (c, ort, &discontiguous))
remove = true;
else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
Thanks,
Tobias,
who really needs to get going with his review of part 3/3!
