This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, OpenACC, 7/8] Multi-dimensional dynamic array support for OpenACC data clauses, libgomp support


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]