Hi Jakub, thanks for the swift review a few weeks ago, and apologies I haven't
been able
to respond sooner.
On 2018/10/16 9:13 PM, Jakub Jelinek wrote:>> +/* Dynamic array related data
structures, interfaces with the compiler. */
+
+struct da_dim {
+ size_t base;
+ size_t length;
+ size_t elem_size;
+ size_t is_array;
+};
+
+struct da_descr_type {
+ void *ptr;
+ size_t ndims;
+ struct da_dim dims[];
+};
Why do you call the non-contiguous arrays dynamic arrays? Is that some OpenACC
term?
I'd also prefix those with gomp_ and it is important to make it clear what
is the ABI type shared with the compiler and what are the internal types.
struct gomp_array_descr would look more natural to me.
Well it's not particularly an OpenACC term, just that non-contiguous arrays are
often multi-dimensional arrays dynamically allocated and created through
(arrays of) pointers.
Are you strongly opposed to this naming? If so, I can adjust this part.
I think the suggested 'gomp_array_descr' identifier looks descriptive, I'll
revise that in an update,
as well as add more comments to better describe its ABI significance with the
compiler.
+ for (i = 0; i < mapnum; i++)
+ {
+ int kind = get_kind (short_mapkind, kinds, i);
+ if (GOMP_MAP_DYNAMIC_ARRAY_P (kind & typemask))
+ {
+ da_data_row_num += gomp_dynamic_array_count_rows (hostaddrs[i]);
+ da_info_num += 1;
+ }
+ }
I'm not really happy by adding several extra loops which will not do
anything in the case there are no non-contiguous arrays being mapped (for
now always for OpenMP (OpenMP 5 has support for non-contigious target update
to/from though) and guess rarely for OpenACC).
Can't you use some flag bit in flags passed to GOMP_target* etc. and do the
above loop only if the compiler indicated there are any?
I originally strived to not have that loop, but because each row in the last
dimension
is mapped as its own target_var_desc, we need to count them at this stage to
allocate
the right number at start. Otherwise a realloc later seems even more ugly...
We currently don't have a suitable flag word argument in GOMP_target*,
GOACC_parallel*, etc.
I am not sure if such a feature warrants changing the interface.
If you are weary of OpenMP being affected, I can add a condition to restrict
such processing
to only (pragma_kind == GOMP_MAP_VARS_OPENACC). Is that okay? (at least before
making any
larger runtime interface adjustments)
+ tgt = gomp_malloc (sizeof (*tgt)
+ + sizeof (tgt->list[0]) * (mapnum + da_data_row_num));
+ tgt->list_count = mapnum + da_data_row_num;
tgt->refcount = pragma_kind == GOMP_MAP_VARS_ENTER_DATA ? 0 : 1;
tgt->device_descr = devicep;
struct gomp_coalesce_buf cbuf, *cbufp = NULL;
@@ -687,6 +863,55 @@ gomp_map_vars (struct gomp_device_descr *devicep, size_t
mapnum,
}
}
+ /* For dynamic arrays. Each data row is one target item, separated from
+ the normal map clause items, hence we order them after mapnum. */
+ for (i = 0, da_index = 0, row_start = 0; i < mapnum; i++)
Even if nothing is in flags, you could just avoid this loop if the previous
loop(s) haven't found any noncontiguous arrays.
I'll add a bit more checking to avoid these cases.
Thanks,
Chung-Lin
