diff mbox series

Avoid -latomic for amdgcn offloading

Message ID nycvar.YFH.7.76.2104211456170.18054@elmra.sevgm.obk
State New
Headers show
Series Avoid -latomic for amdgcn offloading | expand

Commit Message

Richard Biener April 21, 2021, 12:56 p.m. UTC 
libatomic isn't built for amdgcn but reduction-16.c adds it
via -foffload=-latomic when offloading for nvptx is enabled.
The following avoids linker errors when offloading to amdgcn is enabled
as well.

Tested on x86_64-unknown-linux-gnu, OK for trunk and GCC 11 branch?

Thanks.
Richard.

2021-04-21  Richard Biener  <rguenther@suse.de>

libgomp/
	* testsuite/libgomp.c-c++-common/reduction-16.c: Use -latomic
	only on nvptx-none.
---
 libgomp/testsuite/libgomp.c-c++-common/reduction-16.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jeff Law April 21, 2021, 3:10 p.m. UTC | #1 
On 4/21/2021 6:56 AM, Richard Biener wrote:
> libatomic isn't built for amdgcn but reduction-16.c adds it
> via -foffload=-latomic when offloading for nvptx is enabled.
> The following avoids linker errors when offloading to amdgcn is enabled
> as well.
>
> Tested on x86_64-unknown-linux-gnu, OK for trunk and GCC 11 branch?
>
> Thanks.
> Richard.
>
> 2021-04-21  Richard Biener  <rguenther@suse.de>
>
> libgomp/
> 	* testsuite/libgomp.c-c++-common/reduction-16.c: Use -latomic
> 	only on nvptx-none.

OK


Jeff
Thomas Schwinge May 18, 2021, 12:56 p.m. UTC | #2 
Hi!

On 2021-04-21T09:10:52-0600, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On 4/21/2021 6:56 AM, Richard Biener wrote:
>> libatomic isn't built for amdgcn but reduction-16.c adds it
>> via -foffload=-latomic when offloading for nvptx is enabled.

(ACK for that problem.)

>> The following avoids linker errors when offloading to amdgcn is enabled
>> as well.
>>
>> Tested on x86_64-unknown-linux-gnu, OK for trunk and GCC 11 branch?

With which offloading targets?  ;-)

>> 2021-04-21  Richard Biener  <rguenther@suse.de>
>>
>> libgomp/
>>      * testsuite/libgomp.c-c++-common/reduction-16.c: Use -latomic
>>      only on nvptx-none.
>
> OK

Actually, NACK.  ;-|

    -/* { dg-additional-options "-foffload=-latomic" { target offload_target_nvptx } } */
    +/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */

The original options meant that code for *all* offloading targets is
generated.  (... passing '-latomic' to all of them, which is problematic
indeed.)

The new options now mean that code *only* for nvptx offloading is
generated (..., passing '-latomic' to that one), but no other offloading
code is generated anymore; see the 'diff' of 'gcc -v':

     [...]
     Reading specs from [...]
     COLLECT_GCC=[...]
     COLLECT_LTO_WRAPPER=[...]
    -OFFLOAD_TARGET_NAMES=amdgcn-amdhsa:nvptx-none:x86_64-intelmicemul-linux-gnu
    +OFFLOAD_TARGET_NAMES=nvptx-none
     Target: x86_64-pc-linux-gnu
     [...]

Etc.

Thus (presumably) host-fallback execution when running this on a
non-Nvidia GPU system (so not noticeable by default, but also not testing
what is meant to be tested here).

So we need a different solution.


I certainly do agree that the '-foffload' interface is clunky,
overloaded.


Grüße
 Thomas
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
Richard Biener May 18, 2021, 1:57 p.m. UTC | #3 
On Tue, 18 May 2021, Thomas Schwinge wrote:

> Hi!
> 
> On 2021-04-21T09:10:52-0600, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> > On 4/21/2021 6:56 AM, Richard Biener wrote:
> >> libatomic isn't built for amdgcn but reduction-16.c adds it
> >> via -foffload=-latomic when offloading for nvptx is enabled.
> 
> (ACK for that problem.)
> 
> >> The following avoids linker errors when offloading to amdgcn is enabled
> >> as well.
> >>
> >> Tested on x86_64-unknown-linux-gnu, OK for trunk and GCC 11 branch?
> 
> With which offloading targets?  ;-)

Tested with a compiler with nvptx and gcn enabled.  I didn't check
whether the test was executed on GCN ... I have only the host fallback
at runtime anyway.

> >> 2021-04-21  Richard Biener  <rguenther@suse.de>
> >>
> >> libgomp/
> >>      * testsuite/libgomp.c-c++-common/reduction-16.c: Use -latomic
> >>      only on nvptx-none.
> >
> > OK
> 
> Actually, NACK.  ;-|
> 
>     -/* { dg-additional-options "-foffload=-latomic" { target offload_target_nvptx } } */
>     +/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
> 
> The original options meant that code for *all* offloading targets is
> generated.  (... passing '-latomic' to all of them, which is problematic
> indeed.)
> 
> The new options now mean that code *only* for nvptx offloading is
> generated (..., passing '-latomic' to that one), but no other offloading
> code is generated anymore; see the 'diff' of 'gcc -v':
>
>      [...]
>      Reading specs from [...]
>      COLLECT_GCC=[...]
>      COLLECT_LTO_WRAPPER=[...]
>     -OFFLOAD_TARGET_NAMES=amdgcn-amdhsa:nvptx-none:x86_64-intelmicemul-linux-gnu
>     +OFFLOAD_TARGET_NAMES=nvptx-none
>      Target: x86_64-pc-linux-gnu
>      [...]
> 
> Etc.

Doh.  So -foffload=default -foffloat=nvptx-none=-latomic maybe?

But yeah, the interface is clunky.  Maybe add some specs to the GCN
target that throw away -latomic?  (ick)  That said, it would be nice
if -latomic was _added_ by the targets specs when possibly needed,
maybe with -Wl,--as-needed instead of the user being required to add
it himself.

Richard.

> Thus (presumably) host-fallback execution when running this on a
> non-Nvidia GPU system (so not noticeable by default, but also not testing
> what is meant to be tested here).
> 
> So we need a different solution.
> 
> 
> I certainly do agree that the '-foffload' interface is clunky,
> overloaded.
> 
> 
> Grüße
>  Thomas
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
>
Tobias Burnus May 18, 2021, 2:22 p.m. UTC | #4 
On 18.05.21 15:57, Richard Biener wrote:
> Doh.  So -foffload=default -foffloat=nvptx-none=-latomic maybe?

We really need a good documentation for -foffload= and something like
-foffload=default would be good as well – I think we currently only have
'disabled' and an explicit list. (Documentation: cf.
https://gcc.gnu.org/PR67300 )

> That said, it would be nice
> if -latomic was_added_  by the targets specs when possibly needed,
> maybe with -Wl,--as-needed instead of the user being required to add
> it himself.

Last patch was
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556297.html (also
linked to from C99's https://gcc.gnu.org/PR81358 ) – see review comments
in the reply email.

At the end, I did run out of time for this side project and did not
follow up. (The changing the code/configure part should be quick; the
testsuite part is not as straight forward but might or might not be
complicated/lengthy.)

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
diff mbox series

Patch

diff --git a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
index e60fe3664ed..0eea73b144b 100644
--- a/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
+++ b/libgomp/testsuite/libgomp.c-c++-common/reduction-16.c
@@ -1,5 +1,5 @@ 
 /* { dg-do run } */
-/* { dg-additional-options "-foffload=-latomic" { target offload_target_nvptx } } */
+/* { dg-additional-options "-foffload=nvptx-none=-latomic" { target offload_target_nvptx } } */
 
 #include <stdlib.h>