Re: [patch, libgomp] Enable OpenACC GCN testing
Hi!
On 2019-11-14T16:36:38+, Andrew Stubbs wrote:
> This patch adds some necessary bits to enable OpenACC testings for
> amdgcn offloading.
> --- a/libgomp/testsuite/lib/libgomp.exp
> +++ b/libgomp/testsuite/lib/libgomp.exp
> +# Return 1 if at least one AMD GCN board is present, and the AMD GCN device
> +# type is selected by default.
> +
> +proc check_effective_target_openacc_amdgcn_accel_selected { } {
> +if { ![check_effective_target_openacc_amdgcn_accel_present] } {
> + return 0;
> +}
> +global offload_target
> +if { [string match "amdgcn*" $offload_target] } {
> +return 1;
> +}
> +return 0;
> +}
Pushed "[GCN] Streamline
'libgomp/testsuite/lib/libgomp.exp:check_effective_target_openacc_radeon_accel_selected'"
to master branch in commit f9da798ba6348feaada80de04bc72cdf0c4a1f70, see
attached.
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
>From f9da798ba6348feaada80de04bc72cdf0c4a1f70 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Fri, 4 Jun 2021 15:19:35 +0200
Subject: [PATCH] [GCN] Streamline
'libgomp/testsuite/lib/libgomp.exp:check_effective_target_openacc_radeon_accel_selected'
The GCN support that got added in r278935 (commit
83caa34e2a618842e05f59cbb3e2dda93dc23270) "Enable OpenACC GCN testing" was
forked before my r269107 (commit ee332b4a9a19552d160a23155f59b11692d8f07e)
"[libgomp] Clarify difference between offload target, offload plugin, and
OpenACC device type", and didn't later pick up these changes.
No functional change.
libgomp/
* testsuite/lib/libgomp.exp
(check_effective_target_openacc_radeon_accel_selected):
Streamline.
---
libgomp/testsuite/lib/libgomp.exp | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
index 0f4eb6fd4ff..45c78d8510e 100644
--- a/libgomp/testsuite/lib/libgomp.exp
+++ b/libgomp/testsuite/lib/libgomp.exp
@@ -472,11 +472,8 @@ proc check_effective_target_openacc_radeon_accel_selected { } {
if { ![check_effective_target_openacc_radeon_accel_present] } {
return 0;
}
-global offload_target
-if { [string match "amdgcn*" $offload_target] } {
-return 1;
-}
-return 0;
+global openacc_device_type
+return [string match "radeon" $openacc_device_type]
}
# Return 1 if cuda.h and -lcuda are available.
--
2.30.2
Re: [patch, libgomp] Enable OpenACC GCN testing
Hi! On 2019-12-03T12:56:49+, Andrew Stubbs wrote: > On 02/12/2019 14:19, Thomas Schwinge wrote: >> Generally, I'm in favor if you'd consider such a thing (that in principle >> is just a copy/adapt of the existing cases) as obvious to commit (even >> more so with your "amdgcn port" maintainer hat on), especially so given >> that this has been/is blocking you, as Tobias told me more than once. > > I probably will do for incremental tweaks, but I left some stuff in this > one to see if you were awake (that's my story and I'm sticking to it). Haha! ;-P > Here's what I have committed (and I just realized I forgot the new > fangled reviewed-by tag, sorry). No worries. That's just an experiment anyway -- seems to work/have been picked up for glibc, but not so much for GCC. About the alphabetic sorting that I've mentioned: sometimes we do that, sometimes we sort per 'GOMP_DEVICE_*'/'acc_device_t'/whatever ordering, sometimes special cases go to the beginning/end, sometimes it's "in order of appearance", or seemingly random, or any combination of all these. So there isn't a general guideline to follow. However, to make reading the code more easy/enjoyable, what I like to see, is at least some kind of local consistency, instead of just always adding new stuff to the end. (Where the latter might indeed be the right thing to do in certain cases, given that 'GOMP_DEVICE_GCN' is the last one in the list.) Grüße Thomas signature.asc Description: PGP signature
Re: [patch, libgomp] Enable OpenACC GCN testing
On 02/12/2019 14:19, Thomas Schwinge wrote:
Generally, I'm in favor if you'd consider such a thing (that in principle
is just a copy/adapt of the existing cases) as obvious to commit (even
more so with your "amdgcn port" maintainer hat on), especially so given
that this has been/is blocking you, as Tobias told me more than once.
I probably will do for incremental tweaks, but I left some stuff in this
one to see if you were awake (that's my story and I'm sticking to it).
Here's what I have committed (and I just realized I forgot the new
fangled reviewed-by tag, sorry).
Andrew
Enable OpenACC GCN testing.
2019-12-03 Andrew Stubbs
libgomp/
* testsuite/lib/libgomp.exp (offload_target_to_openacc_device_type):
Recognize amdgcn.
(check_effective_target_openacc_amdgcn_accel_present): New proc.
(check_effective_target_openacc_amdgcn_accel_selected): New proc.
* testsuite/libgomp.oacc-c++/c++.exp: Add support for amdgcn.
* testsuite/libgomp.oacc-c/c.exp: Likewise.
* testsuite/libgomp.oacc-fortran/fortran.exp: Likewise.
diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
index 06e3186d966..f52ed7184e4 100644
--- a/libgomp/testsuite/lib/libgomp.exp
+++ b/libgomp/testsuite/lib/libgomp.exp
@@ -318,6 +318,9 @@ proc libgomp_option_proc { option } {
# not supported, and 'host' for offload target 'disable'.
proc offload_target_to_openacc_device_type { offload_target } {
switch -glob $offload_target {
+ amdgcn* {
+ return "gcn"
+ }
disable {
return "host"
}
@@ -479,3 +482,29 @@ proc check_effective_target_hsa_offloading_selected {} {
check_effective_target_hsa_offloading_selected_nocache
}]
}
+
+# Return 1 if at least one AMD GCN board is present.
+
+proc check_effective_target_openacc_amdgcn_accel_present { } {
+return [check_runtime openacc_amdgcn_accel_present {
+ #include
+ int main () {
+ return !(acc_get_num_devices (acc_device_gcn) > 0);
+ }
+} "" ]
+}
+
+# Return 1 if at least one AMD GCN board is present, and the AMD GCN device
+# type is selected by default.
+
+proc check_effective_target_openacc_amdgcn_accel_selected { } {
+if { ![check_effective_target_openacc_amdgcn_accel_present] } {
+ return 0;
+}
+global offload_target
+if { [string match "amdgcn*" $offload_target] } {
+return 1;
+}
+return 0;
+}
+
diff --git a/libgomp/testsuite/libgomp.oacc-c++/c++.exp b/libgomp/testsuite/libgomp.oacc-c++/c++.exp
index dcefa792ca4..c06c2a097e3 100644
--- a/libgomp/testsuite/libgomp.oacc-c++/c++.exp
+++ b/libgomp/testsuite/libgomp.oacc-c++/c++.exp
@@ -88,6 +88,15 @@ if { $lang_test_file_found } {
unsupported "$subdir $offload_target offloading"
continue
}
+ gcn {
+ if { ![check_effective_target_openacc_amdgcn_accel_present] } {
+ # Don't bother; execution testing is going to FAIL.
+ untested "$subdir $offload_target offloading: supported, but hardware not accessible"
+ continue
+ }
+
+ set acc_mem_shared 0
+ }
host {
set acc_mem_shared 1
}
diff --git a/libgomp/testsuite/libgomp.oacc-c/c.exp b/libgomp/testsuite/libgomp.oacc-c/c.exp
index 55cd40f1e99..7f13242fd59 100644
--- a/libgomp/testsuite/libgomp.oacc-c/c.exp
+++ b/libgomp/testsuite/libgomp.oacc-c/c.exp
@@ -51,6 +51,15 @@ foreach offload_target [concat [split $offload_targets ","] "disable"] {
unsupported "$subdir $offload_target offloading"
continue
}
+ gcn {
+ if { ![check_effective_target_openacc_amdgcn_accel_present] } {
+ # Don't bother; execution testing is going to FAIL.
+ untested "$subdir $offload_target offloading: supported, but hardware not accessible"
+ continue
+ }
+
+ set acc_mem_shared 0
+ }
host {
set acc_mem_shared 1
}
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
index 852f372b319..60f0889a07c 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
+++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
@@ -82,6 +82,15 @@ if { $lang_test_file_found } {
unsupported "$subdir $offload_target offloading"
continue
}
+ gcn {
+ if { ![check_effective_target_openacc_amdgcn_accel_present] } {
+ # Don't bother; execution testing is going to FAIL.
+ untested "$subdir $offload_target offloading: supported, but hardware not accessible"
+ continue
+ }
+
+ set acc_mem_shared 0
+ }
host {
set acc_mem_shared 1
}
Re: [patch, libgomp] Enable OpenACC GCN testing
Hi Andrew!
Sorry for the delay.
On 2019-11-14T16:36:38+, Andrew Stubbs wrote:
> This patch adds some necessary bits to enable OpenACC testings for
> amdgcn offloading.
Generally, I'm in favor if you'd consider such a thing (that in principle
is just a copy/adapt of the existing cases) as obvious to commit (even
more so with your "amdgcn port" maintainer hat on), especially so given
that this has been/is blocking you, as Tobias told me more than once.
That said:
> --- a/libgomp/testsuite/lib/libgomp.exp
> +++ b/libgomp/testsuite/lib/libgomp.exp
> @@ -316,6 +316,9 @@ proc offload_target_to_openacc_device_type {
> offload_target } {
> nvptx* {
> return "nvidia"
> }
> + amdgcn* {
> + return "gcn"
> + }
> default {
> error "Unknown offload target: $offload_target"
> }
Maintain alphabetical sorting?
> @@ -444,3 +447,28 @@ proc check_effective_target_hsa_offloading_selected {} {
> check_effective_target_hsa_offloading_selected_nocache
> }]
> }
> +# Return 1 if at least one AMD GCN board is present.
Missing vertical space?
> +proc check_effective_target_openacc_amdgcn_accel_present { } {
> +proc check_effective_target_openacc_amdgcn_accel_selected { } {
I'd also have inserted these maintaining alphabetical sorting, but I see
the existing other stuff also doesn't, so no need to bother.
For all the following three:
> --- a/libgomp/testsuite/libgomp.oacc-c++/c++.exp
> +++ b/libgomp/testsuite/libgomp.oacc-c++/c++.exp
> @@ -106,6 +106,9 @@ if { $lang_test_file_found } {
>
> set acc_mem_shared 0
> }
> + gcn {
> + set acc_mem_shared 0
> + }
> default {
> error "Unknown OpenACC device type: $openacc_device_type
> (offload target: $offload_target)"
> }
> --- a/libgomp/testsuite/libgomp.oacc-c/c.exp
> +++ b/libgomp/testsuite/libgomp.oacc-c/c.exp
> @@ -69,6 +69,9 @@ foreach offload_target [concat [split $offload_targets ","]
> "disable"] {
>
> set acc_mem_shared 0
> }
> + gcn {
> + set acc_mem_shared 0
> + }
> default {
> error "Unknown OpenACC device type: $openacc_device_type (offload
> target: $offload_target)"
> }
> --- a/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
> +++ b/libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
> @@ -94,6 +94,9 @@ if { $lang_test_file_found } {
>
> set acc_mem_shared 0
> }
> + gcn {
> + set acc_mem_shared 0
> + }
> default {
> error "Unknown OpenACC device type: $openacc_device_type
> (offload target: $offload_target)"
> }
..., maintain alphabetical sorting, and please also include the
'untested' check/skip, as done in the 'nvidia' cases.
To record the review effort, please include "Reviewed-by: Thomas Schwinge
" in the commit log, see
.
Grüße
Thomas
signature.asc
Description: PGP signature
Re: [patch, libgomp] Enable OpenACC GCN testing
On 15/11/2019 12:43, Jakub Jelinek wrote: APUs, such as Carizzo are shared memory. DGPUs, such as Fiji and Vega, have their own memory. A DGPU can access host memory, provided that it has been set up just so, but that is very slow, and I don't know of a way to do that without still having to copy the program data into that special region. Ah, that is going to be interesting e.g. for #pragma omp requires unified_shared_memory My plan was that for this the TU constructor would call some libgomp library function that in the end would when loading plugins ensure that only shared memory supporting plugins are loaded. If the gcn plugin will support shared memory only conditionally, we'll need some interfaces to ensure that. This is a problem indeed. Right now the capability has to be declared up front, before the device has even been initialized. In theory, there could be both devices in a single machine, and that would be even worse, but then we don't support machines with heterogeneous DGPUs well either (it works fine for user programs that just use the GPU they're given). We've been basically ignoring it because a) we don't have any APUs to test with, and b) we're being paid to produce a toolchain for supercomputers that will never use APUs (nor mismatched DGPUs). The APU can (pretend to) not use shared memory, of course, so it might Just Work, but I can't be sure. There's also the problem of the XNACK support, which we haven't implemented in the backend. That's specific to APUs, so we'll never need it. You can get some distance without -- there was no mention of it in Honza's original port that targeted Carizzo -- but I don't really know how far. Really, if nobody with an APU wants to fix it, we should just remove the Carizzo support that is there. Andrew
Re: [patch, libgomp] Enable OpenACC GCN testing
On Fri, Nov 15, 2019 at 12:38:06PM +, Andrew Stubbs wrote: > On 15/11/2019 12:21, Jakub Jelinek wrote: > > On Thu, Nov 14, 2019 at 04:36:38PM +, Andrew Stubbs wrote: > > > This patch adds some necessary bits to enable OpenACC testings for amdgcn > > > offloading. > > > > > > The two "check_effective" procedures are not actually needed yet, but > > > later > > > patches to test cases will use them. > > > > > > OK to commit? > > > > I'm surprised by the set acc_mem_shared 0, I thought gcn is a shared memory > > offloading target. > > APUs, such as Carizzo are shared memory. DGPUs, such as Fiji and Vega, have > their own memory. A DGPU can access host memory, provided that it has been > set up just so, but that is very slow, and I don't know of a way to do that > without still having to copy the program data into that special region. Ah, that is going to be interesting e.g. for #pragma omp requires unified_shared_memory My plan was that for this the TU constructor would call some libgomp library function that in the end would when loading plugins ensure that only shared memory supporting plugins are loaded. If the gcn plugin will support shared memory only conditionally, we'll need some interfaces to ensure that. Jakub
Re: [patch, libgomp] Enable OpenACC GCN testing
On 15/11/2019 12:21, Jakub Jelinek wrote: On Thu, Nov 14, 2019 at 04:36:38PM +, Andrew Stubbs wrote: This patch adds some necessary bits to enable OpenACC testings for amdgcn offloading. The two "check_effective" procedures are not actually needed yet, but later patches to test cases will use them. OK to commit? I'm surprised by the set acc_mem_shared 0, I thought gcn is a shared memory offloading target. APUs, such as Carizzo are shared memory. DGPUs, such as Fiji and Vega, have their own memory. A DGPU can access host memory, provided that it has been set up just so, but that is very slow, and I don't know of a way to do that without still having to copy the program data into that special region. The patch is OpenACC only and what do I know about OpenACC? ;) If Thomas is ok with it, the patch is good for trunk. Thanks anyway. Andrew
Re: [patch, libgomp] Enable OpenACC GCN testing
On Thu, Nov 14, 2019 at 04:36:38PM +, Andrew Stubbs wrote: > This patch adds some necessary bits to enable OpenACC testings for amdgcn > offloading. > > The two "check_effective" procedures are not actually needed yet, but later > patches to test cases will use them. > > OK to commit? I'm surprised by the set acc_mem_shared 0, I thought gcn is a shared memory offloading target. The patch is OpenACC only and what do I know about OpenACC? ;) If Thomas is ok with it, the patch is good for trunk. > 2019-11-14 Andrew Stubbs > > libgomp/ > * testsuite/lib/libgomp.exp (offload_target_to_openacc_device_type): > Recognize amdgcn. > (check_effective_target_openacc_amdgcn_accel_present): New proc. > (check_effective_target_openacc_amdgcn_accel_selected): New proc. > * testsuite/libgomp.oacc-c++/c++.exp: Set acc_mem_shared for amdgcn. > * testsuite/libgomp.oacc-c/c.exp: Likewise. > * testsuite/libgomp.oacc-fortran/fortran.exp: Likewise. Jakub
