Re: [RFC PATCH] Coalesce host to device transfers in libgomp

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Jakub!

On Thu, 6 Dec 2018 18:18:56 +0100, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Dec 06, 2018 at 06:01:48PM +0100, Thomas Schwinge wrote:
> > While reviewing Chung-Lin's
> > <https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01428.html> "[PATCH 4/6,
> > OpenACC, libgomp] Async re-work, libgomp/target.c changes", I noticed the
> > following unrelated hunk.  Is that intentional or just an oversight that
> > it hasn't been included in your "gomp_coalesce_buf" changes (quoted below
> > for reference)?
> 
> I believe it is intentional, the coalescing code coalesces only stuff
> allocated by the current gomp_map_vars call, for the link_key case we know
> that is not the case, it is a copy to a file scope data variable in the PTX
> code.

Hmm, I thought this would just copy an address (as opposed to data) from
the host to the device, so that would be fine for coalescing.  But I'm
not familiar with that code, so it's certainly possible that I'm not
understanding this correctly.

> Perhaps we could do the change but pass NULL instead
> of cbufp as the last argument?

Like this?

commit 241027a03b70c788ef94ccf258b799332fb1b20e
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Thu Dec 6 18:53:16 2018 +0100

    Coalesce host to device transfers in libgomp: not for link pointer
    
    2018-12-06  Thomas Schwinge  <thomas@codesourcery.com>
                Jakub Jelinek  <jakub@redhat.com>
    
            libgomp/
            * target.c (gomp_map_vars): Call "gomp_copy_host2dev" instead of
            "devicep->host2dev_func".
---
 libgomp/target.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git libgomp/target.c libgomp/target.c
index 8ebc2a370a16..60f4c96f3908 100644
--- libgomp/target.c
+++ libgomp/target.c
@@ -957,9 +957,11 @@ gomp_map_vars (struct gomp_device_descr *devicep, size_t mapnum,
 		    /* Set link pointer on target to the device address of the
 		       mapped object.  */
 		    void *tgt_addr = (void *) (tgt->tgt_start + k->tgt_offset);
-		    devicep->host2dev_func (devicep->target_id,
-					    (void *) n->tgt_offset,
-					    &tgt_addr, sizeof (void *));
+		    /* We intentionally do not use coalescing here, as it's not
+		       data allocated by the current call to this function.  */
+		    gomp_copy_host2dev (devicep, (void *) n->tgt_offset,
+					&tgt_addr, sizeof (void *), NULL);
+
 		  }
 		array++;
 	      }


Grüße
 Thomas