Re: Fix Intel MIC 'mkoffload' for OpenMP 'requires' (was: [Patch] OpenMP: Move omp requires checks to libgomp)

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Tobias!

On 2022-07-06T13:29:14+0200, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 06.07.22 13:04, Thomas Schwinge wrote:
>> On 2022-06-08T05:56:02+0200, Tobias Burnus <Tobias_Burnus@mentor.com> wrote:
>>> PS: I have not fully tested the intelmic version.
>> As part of my standard testing, I'm reporting that it got completely
>> broken.  ;'-)
>
> Interesting. Because intelmic-mkoffload.cc calls GOMP_offload_register
> and not GOMP_offload_register_ver - and that call path should be unchanged.

True indeed for that code path...

> However, I missed that I had an assert that GCC_OFFLOAD_OMP_REQUIRES_FILE is
> set.

..., but not for that one.

> Thus, an alternative is to change that into an 'if'.
> But I concur that updating intelmic-mkoffload.cc is nicer! Thanks!

ACK.


> Regarding:
>> -! { dg-do link { target { offload_target_nvptx || offload_target_amdgcn } } }
>> +! { dg-do link { target offloading_enabled } }
> This patch looks wrong. We are not interested whether there is an offloading device
> available or not - but whether the offloading compiler is running.
>
> Those are completely independent. Obviously, offloading can be configured but not
> being present. (That's the usual case for testing distro builds but also can
> occur elsewhere.)
> And also the reverse if possible - usually because of -foffload=... but when GCC is
> configured with --enable-offload-defaulted, also other combinations are possible.
>
>
> I think the proper check would be write and use an 'offload_target_any',
> i.e. OFFLOAD_TARGET_NAMES= being present and nonempty.
>
> Cf. check_effective_target_offload_target_nvptx / ..._amdgcn and
> libgomp_check_effective_target_offload_target
> in libgomp/testsuite/lib/libgomp.exp
>
> Possible patch (untested):
>
> # Return 1 if compiling for some offload target(s)
> proc check_effective_target_offload_target_any { } {
>      return [libgomp_check_effective_target_offload_target ""]
> }
>
> At least if I understand the following correctly, "" should work:
>          return [string match "*:$target_name*:*" ":$gcc_offload_targets:"]

:-) Haha, that's actually *exactly* what I had implemented first!  But
then I realized that 'target offloading_enabled' is doing exactly that:
check that offloading compilation is configured -- not that "there is an
offloading device available or not" as you seem to understand?  Or am I
confused there?

I do however agree that (generally) replacing 'target offloading_enabled'
with a new 'target offload_target_any' would seem appropriate (as a
separate patch), because that would also do the right thing when running
libgomp testing with non-default '-foffload=[...]', including
'-foffload=disable'.

For checking "offloading device available" we'd use
'check_effective_target_offload_device[...]'.


Grüße
 Thomas


> Thanks for taking care of the patch fallout!
>
> Tobias
-----------------
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