On Tue, Mar 31, 2015 at 03:52:06PM +0300, Ilya Verbin wrote:
> > What is the reason to register and allocate these one at a time, rather than
> > using one struct target_mem_desc with one tgt->array for all splay tree
> > nodes registered from one image?
> > Perhaps you would just use tgt_start of 0 and tgt_end of 0 too (to make it
> > clear it is special) and just use tgt_offset relative to that (i.e.
> > absolute), but having to malloc each node individually and having to malloc
> > a target_mem_desc for each one sounds expensive.
> > Everything is freed just once anyway, isn't it?
>
> Here is WIP patch, does this look like what you suggested? It works fine with
> functions, however I'm not sure what to do with variables. Will gomp_map_vars
> work when tgt_start and tgt_end are equal to 0?
Can you explain what you are afraid of? The mapped images (both their
mapping and unmapping) are done in pairs, and in a valid program the
addresses shouldn't be already mapped when the image is mapped in etc.
So, for gomp_map_vars, the var allocations should just be the pre-existing
mappings, i.e.
splay_tree_key n = splay_tree_lookup (&mm->splay_tree, &cur_node);
if (n)
{
tgt->list[i] = n;
gomp_map_vars_existing (n, &cur_node, kind & typemask);
}
case and
if (is_target)
{
for (i = 0; i < mapnum; i++)
{
if (tgt->list[i] == NULL)
cur_node.tgt_offset = (uintptr_t) NULL;
else
cur_node.tgt_offset = tgt->list[i]->tgt->tgt_start
+ tgt->list[i]->tgt_offset;
/* FIXME: see above FIXME comment. */
devicep->host2dev_func (devicep->target_id,
(void *) (tgt->tgt_start
+ i * sizeof (void *)),
(void *) &cur_node.tgt_offset,
sizeof (void *));
}
}
at the end. tgt->list[i] will be non-NULL, tgt->list[i]->tgt->tgt_start
will be 0, but tgt->list[i]->tgt_offset will be absolute and so should DTRT.
> + for (i = 0; i < num_vars; i++)
> + {
> + struct addr_pair host_addr;
> + host_addr.start = (uintptr_t) host_var_table[i*2];
> + host_addr.end = host_addr.start + (uintptr_t) host_var_table[i*2+1];
Formatting, spaces around + or *. But, as said earlier, I don't see why
this wouldn't work for variables too.
Jakub
