From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 50000 invoked by alias); 24 Oct 2017 17:39:19 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 49878 invoked by uid 89); 24 Oct 2017 17:39:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.3 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=H*Ad:U*thomas X-HELO: smtp.ispras.ru Received: from bran.ispras.ru (HELO smtp.ispras.ru) (83.149.199.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 24 Oct 2017 17:39:16 +0000 Received: from monopod.intra.ispras.ru (monopod.intra.ispras.ru [10.10.3.121]) by smtp.ispras.ru (Postfix) with ESMTP id 546CF203BF; Tue, 24 Oct 2017 20:39:13 +0300 (MSK) Date: Tue, 24 Oct 2017 17:40:00 -0000 From: Alexander Monakov To: Jakub Jelinek cc: Thomas Schwinge , Martin Jambor , Cesar Philippidis , gcc-patches@gcc.gnu.org Subject: Re: [RFC PATCH] Coalesce host to device transfers in libgomp In-Reply-To: <20171024095527.GJ14653@tucnak> Message-ID: References: <20171024095527.GJ14653@tucnak> User-Agent: Alpine 2.20.13 (LNX 116 2015-12-14) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2017-10/txt/msg01736.txt.bz2 On Tue, 24 Oct 2017, Jakub Jelinek wrote: > loop transfering the addresses or firstprivate_int values to the device > - where we issued mapnum host2dev transfers each just pointer-sized > when we could have just prepared all the pointers in an array and host2dev > copy them all together. Can you please give an example OpenMP code? I thought such variables are just fields of one omp_data_? struct that is copied all at once, but I guess I'm misunderstanding. > Thoughts on this? I need some time to understand the patch well, at the moment I have just a couple superficial comments, below. > --- libgomp/target.c.jj 2017-04-20 14:59:08.296263304 +0200 > +++ libgomp/target.c 2017-10-23 19:08:14.348336118 +0200 > @@ -177,10 +177,77 @@ gomp_device_copy (struct gomp_device_des > } > } > > +struct gomp_map_cache > +{ > + void *buf; > + struct target_mem_desc *tgt; > + size_t *chunks; > + long chunk_cnt; > + long use_cnt; > +}; Would really appreciate comments for meaning of fields here. Also, is the struct properly named? From the patch description I understood it to be a copy coalescing buffer, not a cache. > @@ -449,19 +531,34 @@ gomp_map_vars (struct gomp_device_descr > size_t align = (size_t) 1 << (kind >> rshift); > if (tgt_align < align) > tgt_align = align; > - tgt_size -= (uintptr_t) hostaddrs[first] > - - (uintptr_t) hostaddrs[i]; > + tgt_size -= (uintptr_t) hostaddrs[first] - cur_node.host_start; > tgt_size = (tgt_size + align - 1) & ~(align - 1); > - tgt_size += cur_node.host_end - (uintptr_t) hostaddrs[i]; > + tgt_size += cur_node.host_end - cur_node.host_start; > not_found_cnt += last - i; > for (i = first; i <= last; i++) > - tgt->list[i].key = NULL; > + { > + tgt->list[i].key = NULL; > + switch (get_kind (short_mapkind, kinds, i) & typemask) > + { > + case GOMP_MAP_ALLOC: > + case GOMP_MAP_FROM: > + case GOMP_MAP_FORCE_ALLOC: > + case GOMP_MAP_ALWAYS_FROM: > + break; > + default: > + /* All the others copy data if newly allocated. */ > + gomp_cache_add (&cache, tgt_size - cur_node.host_end > + + (uintptr_t) hostaddrs[i], > + sizes[i]); A similar switch needed to be duplicated below. Would it be appropriate to pass the map kind to gomp_cache_add, or have a thin wrapper around it to have checks for appropriate kinds in one place? Thanks. Alexander