Re: [PATCH 1/5] OpenMP, NVPTX: memcpy[23]D bias correction

[Tobias Burnus <tobias@codesourcery.com>]

Hi Julian & Thomas,

the patch LGTM - and seemingly also Thomas is somewhat fine with it -
and it includes the stand-alone testcase.

* * *

I guess, you don't know the answer to Thomas question, i.e. whether
that's a bug in CUDA or in our use of the CUDA API?

CUDA's spec itself,
https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__MEM.html has
for cuMemcpy2D

‎  void* Start = (void*)((char*)srcHost+srcY*srcPitch + srcXInBytes);

and for cuMemcpy3D

‎  void* Start = (void*)((char*)srcHost+(srcZ*srcHeight+srcY)*srcPitch + srcXInBytes);

Thus, I assume we use it "properly", except that the CUDA writers
probably assumed that one allocates a big chunk of memory and work with
that memory and not just maps a subset.

This might or might not be stated in the manual in the following:
"Memory regions spanning over allocations that are both registered and
not registered with CUDA are not supported and will return
CUDA_ERROR_INVALID_VALUE." – where the question is whether everything
until 'start' really counts as "spanning".

Tobias


On 02.10.23 16:53, Julian Brown wrote:
> On Wed, 27 Sep 2023 00:57:58 +0200
> Thomas Schwinge <thomas@codesourcery.com> wrote:
>
>> On 2023-09-06T02:34:30-0700, Julian Brown <julian@codesourcery.com>
>> wrote:
>>> This patch works around behaviour of the 2D and 3D memcpy
>>> operations in the CUDA driver runtime.  Particularly in Fortran,
>>> the "base pointer" of an array (used for either source or
>>> destination of a host/device copy) may lie outside of data that is
>>> actually stored on the device.  The fix is to make sure that we use
>>> the first element of data to be transferred instead, and adjust
>>> parameters accordingly.
>> Do you (a) have a stand-alone test case for this (that is, not
>> depending on your other pending patches, so that this could go in
>> directly -- together with the before-FAIL test case).
> Thanks for the reply! Here's a version with a stand-alone test case.
>
>> Do you (b)
>> know if is this a bug in our use of the CUDA Driver API or rather in
>> CUDA itself?  If the latter, have you reported this to Nvidia?
> I don't think the CUDA behaviour is *wrong*, as such -- at least to the
> C/C++ way of thinking (or indeed a graphics-oriented way of thinking),
> one would normally think of an array as having a zero-based origin, and
> these 2D/3D memory copies would be intended as a way of updating just a
> part of an array (or texture) that has full duplicate copies on both
> the host and device.  Our use-case just happens to be a bit different,
> both because Fortran (internally) represents an array by a zero-based
> origin but may use 1-based (or whatever-based) indices, and because we
> support partial mappings of host arrays on the device in all three
> supported languages -- which amounts to much the same thing, actually.
>
> That said, it *could* be fixed in CUDA, though probably not in all the
> versions currently deployed out there in the world.  So I guess we'd
> still need a patch like this anyway.
>
> Julian
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955