Hi!

On 2019-11-04T17:35:32+0100, Jakub Jelinek <ja...@redhat.com> wrote:
> On Mon, Nov 04, 2019 at 04:33:27PM +0000, Andrew Stubbs wrote:
>> On 04/11/2019 15:37, Jakub Jelinek wrote:
>> > My preference would be that arch on amdgcn is something like amdgcn or gcn.
>> > I hope the general distinction between arch and isa will be something that
>> > will be discussed next Tuesday on the language committee, so hopefully 
>> > we'll
>> > know more afterwards and can tweak afterwards depending on the outcome.
>> >
>> > Ok with that change.
>>
>> I'm fine with this, too. The OpenMP support should be posted Real Soon Now.
>>
>> We've not been terribly consistent with "gcn" vs. "amdgcn", which is
>> unfortunate, but we are where we are. Most of the API enums have "GCN", so
>> let's use that, for now.
>
> With the way it is defined in the OpenMP spec, there can be actually aliases, 
> so
> having both gcn and amdgcn as supported arch is fine too.

Has this been resolved yet?


On 2019-11-04T16:31:10+0100, Tobias Burnus <tob...@codesourcery.com> wrote:
> This adds gcc/config/gcn/t-omp-device to augment the nvptx, hsa and x86
> host implementation.

> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -1037,6 +1037,10 @@ for tgt in `echo $enable_offload_targets | sed 's/,/ 
> /g'`; do

> +      gcn*-*)
> +     omp_device_property=omp-device-properties-gcn
> +     omp_device_property_tmake_file="${omp_device_property_tmake_file} 
> \$(srcdir)/config/gcn/t-omp-device"
> +     ;;

After some messy debugging (see the commit log), I've pushed as obvious
to master branch in commit b6a0ae1d22c9675f4374c2cb2b5c0833bb1461f1
"[gcn] Fix 'omp-device-properties-gcn' handling", see attached.

Quoting '"$${props}"' as a safety measure didn't solve the problem, as
we're then just being told 'sed: can't read : No such file or directory'
thrice, without any error condition arising -- sloppy shell programming
without proper error checking/handling, as seen so often...  :-| (Not
Tobias' fault here, of course.)  I'm not going to fix up this one case,
as there are so many other pre-existing ones already...

What I don't understand is why this only did hang occasionally but not
all the time...  (What "junk" could there be on 'stdin' to read, or why
would 'stdin' just sometimes be closed, EOF?)

With that fixed, we now get the expected
'build-gcc/gcc/omp-device-properties-gcn', and:

    $ diff -U1 [...] build-gcc/gcc/omp-device-properties.h
    --- [...] 2020-04-23 10:03:38.278178193 +0200
    +++ build-gcc/gcc/omp-device-properties.h       2020-04-23 
22:16:52.472436595 +0200
    @@ -2,2 +2,3 @@
     "amdgcn-amdhsa\0"
    +"gpu\0\0"
     "nvptx-none\0"
    @@ -9,2 +10,3 @@
     "amdgcn-amdhsa\0"
    +"gcn\0\0"
     "nvptx-none\0"
    @@ -16,2 +18,3 @@
     "amdgcn-amdhsa\0"
    +"fiji\0gfx900\0gfx906\0\0"
     "nvptx-none\0"


Doesn't this thing need some testsuite coverage, like
'c-c++-common/gomp/declare-variant-9.c',
'c-c++-common/gomp/declare-variant-10.c'?  (... which would've caught the
problem I've run into here?)


Also, nvptx testsuite coverage could be better, too...  :-|


Grüße
 Thomas


-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander 
Walter
>From b6a0ae1d22c9675f4374c2cb2b5c0833bb1461f1 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Thu, 23 Apr 2020 21:45:34 +0200
Subject: [PATCH] [gcn] Fix 'omp-device-properties-gcn' handling

Fix-up for commit 955cd057454b323419e06affa7df7d59dc3cd1fb "Add
gcc/config/gcn/t-omp-device for OpenMP declare variant kind/arch/isa".

With AMD GCN offloading configured, I'm seeing occasional GCC build hangs.
I've now captured and analyzed one of them:

    $ ps -f
    UID        PID  PPID  C STIME TTY          TIME CMD
    [...]
    tschwing  5113  4508  0 20:24 pts/5    00:00:00 /bin/sh -c rm -f tmp-omp-device-properties.h; \ for kind in kind arch isa; do \   echo 'const char omp_offload_device_'${kind}'[] = ' \     >> tmp-omp-device-properties.h; \   for prop in no
    tschwing  5126  5113  0 20:24 pts/5    00:00:00 sed -n s/^kind: //p
    tschwing  5127  5113  0 20:24 pts/5    00:00:00 sed s/[[:blank:]]/ /g;s/  */ /g;s/^ //;s/ $//;s/ /\\0/g;s/^/"/;s/$/\\0\\0"/
    [...]
    $ pstree -p $$
    [...]---sh(5113)-+-sed(5126)
                     `-sed(5127)
    $ ls -lrt build-gcc/gcc/*omp-device*
    -rw-r--r-- 1 tschwing eeg  39 Apr 23 20:24 build-gcc/gcc/omp-device-properties-nvptx
    -rw-r--r-- 1 tschwing eeg 634 Apr 23 20:24 build-gcc/gcc/omp-device-properties-i386
    -rw-r--r-- 1 tschwing eeg  58 Apr 23 20:24 build-gcc/gcc/tmp-omp-device-properties.h

Notably missing is the 'omp-device-properties-gcn' file...

    $ grep ^ build-gcc/gcc/*omp-device*
    build-gcc/gcc/omp-device-properties-i386:kind: cpu
    build-gcc/gcc/omp-device-properties-i386:arch: x86 x86_64 i386 i486 i586 i686 ia32
    build-gcc/gcc/omp-device-properties-i386:isa: sse4 cx16 [...]
    build-gcc/gcc/omp-device-properties-nvptx:kind: gpu
    build-gcc/gcc/omp-device-properties-nvptx:arch: nvptx
    build-gcc/gcc/omp-device-properties-nvptx:isa: sm_30 sm_35
    build-gcc/gcc/tmp-omp-device-properties.h:const char omp_offload_device_kind[] =
    build-gcc/gcc/tmp-omp-device-properties.h:"amdgcn-amdhsa\0"

..., which we here seem to be intending to fill into
'tmp-omp-device-properties.h'.

    $ grep ^omp_device_properties\ = build-gcc/gcc/Makefile
    omp_device_properties =  amdgcn-amdhsa= nvptx-none=omp-device-properties-nvptx x86_64-intelmicemul-linux-gnu=omp-device-properties-i386

Given the 's-omp-device-properties-h' Makefile rule, indeed there is an
unescaped '$${props}', which is meant to be the filename following the equals
sign -- but there is none for 'amdgcn-amdhsa=', so this tries to read from
'stdin'!

The real problem of course is elsewhere.

	gcc/
	* configure.ac <$enable_offload_targets>: 'amdgcn' is 'gcn'.
	* configure: Regenerate.
---
 gcc/ChangeLog    | 3 +++
 gcc/configure    | 2 +-
 gcc/configure.ac | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ef851ef84626..85d1c2b6f758 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,8 @@
 2020-04-29  Thomas Schwinge  <tho...@codesourcery.com>
 
+	* configure.ac <$enable_offload_targets>: 'amdgcn' is 'gcn'.
+	* configure: Regenerate.
+
 	PR target/94279
 	* rtlanal.c (set_noop_p): Handle non-constant selectors.
 
diff --git a/gcc/configure b/gcc/configure
index 9e22c5a286f6..83101072aea0 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -7924,7 +7924,7 @@ for tgt in `echo $enable_offload_targets | sed 's/,/ /g'`; do
 	omp_device_property=omp-device-properties-i386
 	omp_device_property_tmake_file="${omp_device_property_tmake_file} \$(srcdir)/config/i386/t-omp-device"
 	;;
-      gcn*-*)
+      amdgcn*-*)
 	omp_device_property=omp-device-properties-gcn
 	omp_device_property_tmake_file="${omp_device_property_tmake_file} \$(srcdir)/config/gcn/t-omp-device"
 	;;
diff --git a/gcc/configure.ac b/gcc/configure.ac
index cd62312b813c..b604047ae456 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -1049,7 +1049,7 @@ for tgt in `echo $enable_offload_targets | sed 's/,/ /g'`; do
 	omp_device_property=omp-device-properties-i386
 	omp_device_property_tmake_file="${omp_device_property_tmake_file} \$(srcdir)/config/i386/t-omp-device"
 	;;
-      gcn*-*)
+      amdgcn*-*)
 	omp_device_property=omp-device-properties-gcn
 	omp_device_property_tmake_file="${omp_device_property_tmake_file} \$(srcdir)/config/gcn/t-omp-device"
 	;;
-- 
2.26.2

Reply via email to