* [patch, libgomp] Re-factor GOMP_MAP_POINTER handling
@ 2015-04-21 12:22 Chung-Lin Tang
2015-05-11 11:19 ` Chung-Lin Tang
0 siblings, 1 reply; 5+ messages in thread
From: Chung-Lin Tang @ 2015-04-21 12:22 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Thomas Schwinge
[-- Attachment #1: Type: text/plain, Size: 595 bytes --]
Hi,
while investigating some issues in the variable mapping code, I observed
that the GOMP_MAP_POINTER handling is essentially duplicated under the PSET case.
This patch abstracts and unifies the handling code, basically just a cleanup
patch. Ran libgomp tests to ensure no regressions, ok for trunk?
Thanks,
Chung-Lin
2015-04-21 Chung-Lin Tang <cltang@codesourcery.com>
libgomp/
* target.c (gomp_map_pointer): New function abstracting out
GOMP_MAP_POINTER handling.
(gomp_map_vars): Remove GOMP_MAP_POINTER handling code and use
gomp_map_pointer().
[-- Attachment #2: gomp-map-pointer.patch --]
[-- Type: text/x-patch, Size: 6493 bytes --]
Index: target.c
===================================================================
--- target.c (revision 448412)
+++ target.c (working copy)
@@ -163,6 +163,60 @@ get_kind (bool is_openacc, void *kinds, int idx)
: ((unsigned char *) kinds)[idx];
}
+static void
+gomp_map_pointer (struct target_mem_desc *tgt, uintptr_t host_ptr,
+ uintptr_t target_offset, uintptr_t bias)
+{
+ struct gomp_device_descr *devicep = tgt->device_descr;
+ struct splay_tree_s *mem_map = &devicep->mem_map;
+ struct splay_tree_key_s cur_node;
+
+ cur_node.host_start = host_ptr;
+ if (cur_node.host_start == (uintptr_t) NULL)
+ {
+ cur_node.tgt_offset = (uintptr_t) NULL;
+ /* FIXME: see comment about coalescing host/dev transfers below. */
+ devicep->host2dev_func (devicep->target_id,
+ (void *) (tgt->tgt_start + target_offset),
+ (void *) &cur_node.tgt_offset,
+ sizeof (void *));
+ return;
+ }
+ /* Add bias to the pointer value. */
+ cur_node.host_start += bias;
+ cur_node.host_end = cur_node.host_start + 1;
+ splay_tree_key n = splay_tree_lookup (mem_map, &cur_node);
+ if (n == NULL)
+ {
+ /* Could be possibly zero size array section. */
+ cur_node.host_end--;
+ n = splay_tree_lookup (mem_map, &cur_node);
+ if (n == NULL)
+ {
+ cur_node.host_start--;
+ n = splay_tree_lookup (mem_map, &cur_node);
+ cur_node.host_start++;
+ }
+ }
+ if (n == NULL)
+ {
+ gomp_mutex_unlock (&devicep->lock);
+ gomp_fatal ("Pointer target of array section wasn't mapped");
+ }
+ cur_node.host_start -= n->host_start;
+ cur_node.tgt_offset
+ = n->tgt->tgt_start + n->tgt_offset + cur_node.host_start;
+ /* At this point tgt_offset is target address of the
+ array section. Now subtract bias to get what we want
+ to initialize the pointer with. */
+ cur_node.tgt_offset -= bias;
+ /* FIXME: see comment about coalescing host/dev transfers below. */
+ devicep->host2dev_func (devicep->target_id,
+ (void *) (tgt->tgt_start + target_offset),
+ (void *) &cur_node.tgt_offset,
+ sizeof (void *));
+}
+
attribute_hidden struct target_mem_desc *
gomp_map_vars (struct gomp_device_descr *devicep, size_t mapnum,
void **hostaddrs, void **devaddrs, size_t *sizes, void *kinds,
@@ -336,54 +390,8 @@ gomp_map_vars (struct gomp_device_descr *devicep,
k->host_end - k->host_start);
break;
case GOMP_MAP_POINTER:
- cur_node.host_start
- = (uintptr_t) *(void **) k->host_start;
- if (cur_node.host_start == (uintptr_t) NULL)
- {
- cur_node.tgt_offset = (uintptr_t) NULL;
- /* FIXME: see above FIXME comment. */
- devicep->host2dev_func (devicep->target_id,
- (void *) (tgt->tgt_start
- + k->tgt_offset),
- (void *) &cur_node.tgt_offset,
- sizeof (void *));
- break;
- }
- /* Add bias to the pointer value. */
- cur_node.host_start += sizes[i];
- cur_node.host_end = cur_node.host_start + 1;
- n = splay_tree_lookup (mem_map, &cur_node);
- if (n == NULL)
- {
- /* Could be possibly zero size array section. */
- cur_node.host_end--;
- n = splay_tree_lookup (mem_map, &cur_node);
- if (n == NULL)
- {
- cur_node.host_start--;
- n = splay_tree_lookup (mem_map, &cur_node);
- cur_node.host_start++;
- }
- }
- if (n == NULL)
- {
- gomp_mutex_unlock (&devicep->lock);
- gomp_fatal ("Pointer target of array section "
- "wasn't mapped");
- }
- cur_node.host_start -= n->host_start;
- cur_node.tgt_offset = n->tgt->tgt_start + n->tgt_offset
- + cur_node.host_start;
- /* At this point tgt_offset is target address of the
- array section. Now subtract bias to get what we want
- to initialize the pointer with. */
- cur_node.tgt_offset -= sizes[i];
- /* FIXME: see above FIXME comment. */
- devicep->host2dev_func (devicep->target_id,
- (void *) (tgt->tgt_start
- + k->tgt_offset),
- (void *) &cur_node.tgt_offset,
- sizeof (void *));
+ gomp_map_pointer (tgt, (uintptr_t) *(void **) k->host_start,
+ k->tgt_offset, sizes[i]);
break;
case GOMP_MAP_TO_PSET:
/* FIXME: see above FIXME comment. */
@@ -405,58 +413,12 @@ gomp_map_vars (struct gomp_device_descr *devicep,
{
tgt->list[j] = k;
k->refcount++;
- cur_node.host_start
- = (uintptr_t) *(void **) hostaddrs[j];
- if (cur_node.host_start == (uintptr_t) NULL)
- {
- cur_node.tgt_offset = (uintptr_t) NULL;
- /* FIXME: see above FIXME comment. */
- devicep->host2dev_func (devicep->target_id,
- (void *) (tgt->tgt_start + k->tgt_offset
- + ((uintptr_t) hostaddrs[j]
- - k->host_start)),
- (void *) &cur_node.tgt_offset,
- sizeof (void *));
- i++;
- continue;
- }
- /* Add bias to the pointer value. */
- cur_node.host_start += sizes[j];
- cur_node.host_end = cur_node.host_start + 1;
- n = splay_tree_lookup (mem_map, &cur_node);
- if (n == NULL)
- {
- /* Could be possibly zero size array section. */
- cur_node.host_end--;
- n = splay_tree_lookup (mem_map, &cur_node);
- if (n == NULL)
- {
- cur_node.host_start--;
- n = splay_tree_lookup (mem_map, &cur_node);
- cur_node.host_start++;
- }
- }
- if (n == NULL)
- {
- gomp_mutex_unlock (&devicep->lock);
- gomp_fatal ("Pointer target of array section "
- "wasn't mapped");
- }
- cur_node.host_start -= n->host_start;
- cur_node.tgt_offset = n->tgt->tgt_start
- + n->tgt_offset
- + cur_node.host_start;
- /* At this point tgt_offset is target address of the
- array section. Now subtract bias to get what we
- want to initialize the pointer with. */
- cur_node.tgt_offset -= sizes[j];
- /* FIXME: see above FIXME comment. */
- devicep->host2dev_func (devicep->target_id,
- (void *) (tgt->tgt_start + k->tgt_offset
- + ((uintptr_t) hostaddrs[j]
- - k->host_start)),
- (void *) &cur_node.tgt_offset,
- sizeof (void *));
+ gomp_map_pointer (tgt,
+ (uintptr_t) *(void **) hostaddrs[j],
+ k->tgt_offset
+ + ((uintptr_t) hostaddrs[j]
+ - k->host_start),
+ sizes[j]);
i++;
}
break;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, libgomp] Re-factor GOMP_MAP_POINTER handling
2015-04-21 12:22 [patch, libgomp] Re-factor GOMP_MAP_POINTER handling Chung-Lin Tang
@ 2015-05-11 11:19 ` Chung-Lin Tang
2015-05-21 8:54 ` Chung-Lin Tang
0 siblings, 1 reply; 5+ messages in thread
From: Chung-Lin Tang @ 2015-05-11 11:19 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Thomas Schwinge
Ping.
On 2015/4/21 08:21 PM, Chung-Lin Tang wrote:
> Hi,
> while investigating some issues in the variable mapping code, I observed
> that the GOMP_MAP_POINTER handling is essentially duplicated under the PSET case.
> This patch abstracts and unifies the handling code, basically just a cleanup
> patch. Ran libgomp tests to ensure no regressions, ok for trunk?
>
> Thanks,
> Chung-Lin
>
> 2015-04-21 Chung-Lin Tang <cltang@codesourcery.com>
>
> libgomp/
> * target.c (gomp_map_pointer): New function abstracting out
> GOMP_MAP_POINTER handling.
> (gomp_map_vars): Remove GOMP_MAP_POINTER handling code and use
> gomp_map_pointer().
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, libgomp] Re-factor GOMP_MAP_POINTER handling
2015-05-11 11:19 ` Chung-Lin Tang
@ 2015-05-21 8:54 ` Chung-Lin Tang
2015-05-21 13:11 ` Thomas Schwinge
0 siblings, 1 reply; 5+ messages in thread
From: Chung-Lin Tang @ 2015-05-21 8:54 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek, Thomas Schwinge
Ping x2.
On 15/5/11 7:19 PM, Chung-Lin Tang wrote:
> Ping.
>
> On 2015/4/21 08:21 PM, Chung-Lin Tang wrote:
>> Hi,
>> while investigating some issues in the variable mapping code, I observed
>> that the GOMP_MAP_POINTER handling is essentially duplicated under the PSET case.
>> This patch abstracts and unifies the handling code, basically just a cleanup
>> patch. Ran libgomp tests to ensure no regressions, ok for trunk?
>>
>> Thanks,
>> Chung-Lin
>>
>> 2015-04-21 Chung-Lin Tang <cltang@codesourcery.com>
>>
>> libgomp/
>> * target.c (gomp_map_pointer): New function abstracting out
>> GOMP_MAP_POINTER handling.
>> (gomp_map_vars): Remove GOMP_MAP_POINTER handling code and use
>> gomp_map_pointer().
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, libgomp] Re-factor GOMP_MAP_POINTER handling
2015-05-21 8:54 ` Chung-Lin Tang
@ 2015-05-21 13:11 ` Thomas Schwinge
2015-05-21 13:38 ` Jakub Jelinek
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Schwinge @ 2015-05-21 13:11 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Chung-Lin Tang, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]
Hi!
Jakub, for avoidance of doubt, the proposed refactoring makes sense to
me, but does need your approval:
On Thu, 21 May 2015 16:30:40 +0800, Chung-Lin Tang <cltang@codesourcery.com> wrote:
> Ping x2.
>
> On 15/5/11 7:19 PM, Chung-Lin Tang wrote:
> > Ping.
> >
> > On 2015/4/21 08:21 PM, Chung-Lin Tang wrote:
> >> Hi,
> >> while investigating some issues in the variable mapping code, I observed
> >> that the GOMP_MAP_POINTER handling is essentially duplicated under the PSET case.
> >> This patch abstracts and unifies the handling code, basically just a cleanup
> >> patch. Ran libgomp tests to ensure no regressions, ok for trunk?
> >>
> >> Thanks,
> >> Chung-Lin
> >>
> >> 2015-04-21 Chung-Lin Tang <cltang@codesourcery.com>
> >>
> >> libgomp/
> >> * target.c (gomp_map_pointer): New function abstracting out
> >> GOMP_MAP_POINTER handling.
> >> (gomp_map_vars): Remove GOMP_MAP_POINTER handling code and use
> >> gomp_map_pointer().
Grüße,
Thomas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, libgomp] Re-factor GOMP_MAP_POINTER handling
2015-05-21 13:11 ` Thomas Schwinge
@ 2015-05-21 13:38 ` Jakub Jelinek
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Jelinek @ 2015-05-21 13:38 UTC (permalink / raw)
To: Thomas Schwinge; +Cc: Chung-Lin Tang, gcc-patches
On Thu, May 21, 2015 at 03:00:16PM +0200, Thomas Schwinge wrote:
> Jakub, for avoidance of doubt, the proposed refactoring makes sense to
> me, but does need your approval:
This is ok for trunk.
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-21 13:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-21 12:22 [patch, libgomp] Re-factor GOMP_MAP_POINTER handling Chung-Lin Tang
2015-05-11 11:19 ` Chung-Lin Tang
2015-05-21 8:54 ` Chung-Lin Tang
2015-05-21 13:11 ` Thomas Schwinge
2015-05-21 13:38 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).