https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92028
Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2019-10-08
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
Ever confirmed|0 |1
--- Comment #2 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #1)
> Looking at that commit, what I've commited in target.c is certainly not what
> I meant to commit, which was something like (untested):
Thanks, that does resolves the issue. :-)
Now, one very general comment:
> --- libgomp/target.c.jj 2019-10-07 13:09:07.038253353 +0200
> +++ libgomp/target.c 2019-10-08 15:19:16.249439849 +0200
@@ -593,6 +593,19 @@ gomp_map_vars_internal (struct gomp_devi
> tgt->list[i].key = NULL;
> if (!not_found_cnt)
> {
> + cur_node.host_start = (uintptr_t) hostaddrs[i];
> + cur_node.host_end = cur_node.host_start;
> + splay_tree_key n = gomp_map_lookup (mem_map, &cur_node);
> + if (n == NULL)
> + {
> + gomp_mutex_unlock (&devicep->lock);
> + gomp_fatal ("use_device_ptr pointer wasn't mapped");
> + }
> + cur_node.host_start -= n->host_start;
> + hostaddrs[i]
> + = (void *) (n->tgt->tgt_start + n->tgt_offset
> + + cur_node.host_start);
> + tgt->list[i].offset = ~(uintptr_t) 0;
> }
> else
> tgt->list[i].offset = 0;
So, with some careful thinking/digging one is of course able to extract:
> Essentially, if nothing is so far queued to be mapped, we can try to resolve
> USE_DEVICE_PTR the way we did before, but if there is something queued,
> because the middle-end for OpenMP 5.0 ensures that map clauses go before
> use_device_* clauses, we need to wait and handle it only later.
... such a rationale out of the actual GCC source code -- but what's the reason
that you're not adding such comments directly to the code?
Not just here, but generally I find that your patch submission emails and PR
comments very often contain very valuable information, but that's just in email
but not in the actual GCC source code -- so that information is not readily
accessible when reading the code. Which is a pity.
Can I suggest that we all -- including you :-P -- help to make this complex OMP
gimplification/lowering/expanding/libgomp code better understandable and
maintainable by documenting it better?
