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] |
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] |