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


On 2018/12/6 10:43 PM, Jakub Jelinek wrote:
On Thu, Dec 06, 2018 at 10:19:43PM +0800, Chung-Lin Tang wrote:
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.

The way how those arrays are created (and it doesn't have to be dynamically
allocated) doesn't affect their representation.
There are various terms that describe various data structures, like Iliffe
vectors, jagged/ragged arrays, dope vectors.
I guess it depends on what kind of data structures does this new framework
support, if the Iliffe vectors (arrays of pointers), or just flat but
strided arrays, etc.

+  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)

That will still cost you doing that loop for OpenACC constructs that don't
have any of these non-contiguous arrays.  GOMP_target_ext has flags
argument, but GOACC_paralel_keyed doesn't.  It has ... and you could perhaps
encode some flag in there.  Or, could these array descriptors be passed
first in the list of vars, so instead of a loop to check for these you could
just check the first one?

Hi Jakub,
I have revised the patch to rename the main struct da_* types into struct gomp_array_* and
added more detailed descriptions in the comments (though admittedly the "dynamic array" term
is not purged completely).

I have opted for the place-at-start-of-chain route, this should avoid all the tests and
additional iterating when such arrays are not used. There's also another omp-low.c update in
another patch.

Besides the revised whole patch, I have also attached a v1-v2 diff showing the changes in between.
Tested with offloading to ensure no regressions.

Thanks,
Chung-Lin





Attachment: openacc-da-07.libgomp-target.v2.patch
Description: Text document

Attachment: 07-target.c.v1-v2.diff
Description: Text document


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