Re: [PATCH] libgomp, OpenMP: Fix issue for omp_get_device_num on gfx targets.

[Marcel Vollweiler <marcel@codesourcery.com>]

Hi Thomas,

Am 18.01.2022 um 13:25 schrieb Thomas Schwinge:
> Hi!
>
> Maybe I'm just totally confused -- as so often ;-) -- but things seem
> strange here:
>
> On 2022-01-12T10:43:05+0100, Marcel Vollweiler <marcel@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?

Yes, as far as I understood.

>
>> 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?

Good question. In contrast to nvptx I observed that __gomp_device_num is
not part of the offload image which we read out in
GOMP_OFFLOAD_load_image ("if (status != HSA_STATUS_SUCCESS)" in
libgomp/plugin/plugin-gcn.c returns false). I validated it with some
additional output in the if-branches.

>
>> 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?
>
>> 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?

Actually, I don't see the reason for the different behaviour for nvptx.
I just tested that for nvptx the correct device number is returned by
omp_get_device_num on the device - also if we have more than one device.

>
> Still, should 'static' be removed here, too?

I wouldn't suggest unless it is really necessary? I mean, there we don't
have any issue. Although I aggree with Andrew that we could combine both
icv-device.c files into one common file.

Marcel
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955