On 18/01/2022 12:25, Thomas Schwinge wrote:
Hi!
Maybe I'm just totally confused -- as so often ;-) -- but things seem
strange here:
On 2022-01-12T10:43:05+0100, Marcel Vollweiler <mar...@codesourcery.com> wrote:
Currently omp_get_device_num does not work on gcn targets with more than
one offload device. The reason is that GOMP_DEVICE_NUM_VAR
I understand the 'GOMP_DEVICE_NUM_VAR' "macro indirection" is so that we
define the actual symbol name ('__gomp_device_num') in one place
('libgomp/libgomp-plugin.h'), and then use it (via macro expansion) in
several places, right?
Right. I don't know what the motivation behind that design was, but
Marcel has not invented this.
is static in
icv-device.c and thus "__gomp_device_num" is not visible in the offload
image.
That behavior seems correct -- but undesired indeed?
This patch removes "static" such that "__gomp_device_num" is now part of
the offload image and can now be found in GOMP_OFFLOAD_load_image in the
plugin.
That seems correct?
Or, is there a reason to have it 'static', say, so that several such
local variables can co-exist, instead of just one global one?
If there were several similar symbols "coexisting" then the libgomp
plugin would not know which to update at kernel launch time, and
different parts of libgomp would see different values for the same ICV.
I can't think of any reason why we'd want that?
My guess is that it started out as a static variable inside a function
and was then moved to file-scope without removing the keyword.
This is not an issue for nvptx. There, "__gomp_device_num" is in the
offload image even with "static".
That's unexpected then, and should be looked into?
I suspect it's just because NVPTX doesn't use ELF at link time. Multiple
static variables with the same name in different translation units might
even be broken?
Still, should 'static' be removed here, too?
I would have thought so. In fact there's no need for nvptx and amdgcn to
have independent icv-device.c files, given that they must both implement
the same features and there's nothing architecture-specific going on
here (thus far).
Andrew
