[PATCH][testsuite, nvptx] Add effective target sync_int_long_stack

[PATCH][testsuite, nvptx] Add effective target sync_int_long_stack

[Tom de Vries]

Hi,

The nvptx target currently doesn't support effective target sync_int_long,
although it has support for 32-bit and 64-bit atomic.

When enabling sync_int_long for nvptx, we run into a failure in
gcc.dg/pr86314.c:
...
 nvptx-run: error getting kernel result: operation not supported on \
   global/shared address space
...
due to a ptx restriction:  accesses to local memory are illegal, and the
test-case does an atomic operation on a stack address, which is mapped to
local memory.

Fix this by adding a target sync_int_long_stack, wich returns false for nvptx,
which can be used to mark test-cases that require sync_int_long support for
stack address.

Build on nvptx and tested with make check-gcc.

OK for trunk?

Thanks,
- Tom

[testsuite, nvptx] Add effective target sync_int_long_stack

gcc/testsuite/ChangeLog:

	PR target/96494
	* lib/target-supports.exp (check_effective_target_sync_int_long):
	Return 1 for nvptx.
	(check_effective_target_sync_int_long_stack): New proc.
	* gcc.dg/pr86314.c: Require effective target sync_int_long_stack.

---
 gcc/testsuite/gcc.dg/pr86314.c        |  2 +-
 gcc/testsuite/lib/target-supports.exp | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/pr86314.c b/gcc/testsuite/gcc.dg/pr86314.c
index 8962a3cf2ff..565fb02eee2 100644
--- a/gcc/testsuite/gcc.dg/pr86314.c
+++ b/gcc/testsuite/gcc.dg/pr86314.c
@@ -1,5 +1,5 @@
 // PR target/86314
-// { dg-do run { target sync_int_long } }
+// { dg-do run { target sync_int_long_stack } }
 // { dg-options "-O2" }
 
 __attribute__((noinline, noclone)) unsigned long
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index e79015b4d54..a870b1de275 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -7704,7 +7704,16 @@ proc check_effective_target_sync_int_long { } {
 	     || [istarget cris-*-*]
 	     || ([istarget sparc*-*-*] && [check_effective_target_sparc_v9])
 	     || ([istarget arc*-*-*] && [check_effective_target_arc_atomic])
-	     || [check_effective_target_mips_llsc] }}]
+	     || [check_effective_target_mips_llsc]
+	     || [istarget nvptx*-*-*]
+	 }}]
+}
+
+proc check_effective_target_sync_int_long_stack { } {
+    return [check_cached_effective_target sync_int_long_stack {
+      expr { ![istarget nvptx*-*-*]
+	     && [check_effective_target_sync_int_long]	     
+	 }}]
 }
 
 # Return 1 if the target supports atomic operations on "char" and "short".

Re: [PATCH][testsuite, nvptx] Add effective target sync_int_long_stack

[Mike Stump]

On Aug 12, 2020, at 6:57 AM, Tom de Vries <tdevries@suse.de> wrote:
> 
> The nvptx target currently doesn't support effective target sync_int_long,
> although it has support for 32-bit and 64-bit atomic.
> 
> When enabling sync_int_long for nvptx, we run into a failure in
> gcc.dg/pr86314.c:
> ...
> nvptx-run: error getting kernel result: operation not supported on \
>   global/shared address space
> ...
> due to a ptx restriction:  accesses to local memory are illegal, and the
> test-case does an atomic operation on a stack address, which is mapped to
> local memory.
> 
> Fix this by adding a target sync_int_long_stack, wich returns false for nvptx,
> which can be used to mark test-cases that require sync_int_long support for
> stack address.
> 
> Build on nvptx and tested with make check-gcc.
> 
> OK for trunk?

Ok.

Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812] (was: [PATCH][testsuite, nvptx] Add effective target sync_int_long_stack)

[Thomas Schwinge]

Hi!

On 2020-08-12T15:57:23+0200, Tom de Vries <tdevries@suse.de> wrote:
> When enabling sync_int_long for nvptx, we run into a failure in
> gcc.dg/pr86314.c:
> ...
>  nvptx-run: error getting kernel result: operation not supported on \
>    global/shared address space
> ...
> due to a ptx restriction:  accesses to local memory are illegal, and the
> test-case does an atomic operation on a stack address, which is mapped to
> local memory.

Now, that problem also is easy to reproduce in an OpenACC/nvptx
offloading setting.  (..., as Julian had reported and analyzed in an
internal "Date: Fri, 31 May 2019 00:29:17 +0100" email, similar to Tom's
comments in PR96494 "[nvptx] Enable effective target sync_int_long" and
PR97444 "[nvptx] stack atomics".)

> Fix this by adding a target sync_int_long_stack, wich returns false for nvptx,
> which can be used to mark test-cases that require sync_int_long support for
> stack address.

Similar to PR97444 "[nvptx] stack atomics", such a conditional is of
course not applicable for the OpenACC implementation, so to at least
document/test/XFAIL nvptx offloading: PR83812 "operation not supported
on global/shared address space", I've now pushed to master branch
"Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812]" in
commit 1467100fc72562a59f70cdd4e05f6c810d1fadcc, see attached.


And then, I got back testresults from one more system, and I've filed
<https://gcc.gnu.org/PR100678> "[OpenACC/nvptx]
'libgomp.oacc-c-c++-common/private-atomic-1.c' FAILs (differently) in
certain configurations"...  :-\

As it's not clear yet what the conditions are, I cannot come up with a
selector to (differently) XFAIL that one.


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

Re: Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812] (was: [PATCH][testsuite, nvptx] Add effective target sync_int_long_stack)

[Thomas Schwinge]

Hi Tom!

On 2021-05-19T14:56:17+0200, I wrote:
> On 2020-08-12T15:57:23+0200, Tom de Vries <tdevries@suse.de> wrote:
>> When enabling sync_int_long for nvptx, we run into a failure in
>> gcc.dg/pr86314.c:
>> ...
>>  nvptx-run: error getting kernel result: operation not supported on \
>>    global/shared address space
>> ...
>> due to a ptx restriction:  accesses to local memory are illegal, and the
>> test-case does an atomic operation on a stack address, which is mapped to
>> local memory.
>
> Now, that problem also is easy to reproduce in an OpenACC/nvptx
> offloading setting.  (..., as Julian had reported and analyzed in an
> internal "Date: Fri, 31 May 2019 00:29:17 +0100" email, similar to Tom's
> comments in PR96494 "[nvptx] Enable effective target sync_int_long" and
> PR97444 "[nvptx] stack atomics".)
>
>> Fix this by adding a target sync_int_long_stack, wich returns false for nvptx,
>> which can be used to mark test-cases that require sync_int_long support for
>> stack address.

Shouldn't this now again be reverted/removed after your
commit e0451f93d9faa13495132f4e246e9bef30b51417
"[nvptx] Add some support for .local atomics"?
(But I haven't looked in detail.)


Is PR83812 "nvptx-run: error getting kernel result: operation not
supported on global/shared address space" resolved then?

Or, because of:

| This only solves some cases that are detected at compile-time.

... not completely resolved?

There's also still PR97444 "[nvptx] stack atomics" open.


> Similar to PR97444 "[nvptx] stack atomics", such a conditional is of
> course not applicable for the OpenACC implementation, so to at least
> document/test/XFAIL nvptx offloading: PR83812 "operation not supported
> on global/shared address space", I've now pushed to master branch
> "Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812]" in
> commit 1467100fc72562a59f70cdd4e05f6c810d1fadcc, see attached.

... which later got replicated in other test cases --
all of which I confirm are resolved and un-XFAILed with
commit e0451f93d9faa13495132f4e246e9bef30b51417
"[nvptx] Add some support for .local atomics".


> And then, I got back testresults from one more system, and I've filed
> <https://gcc.gnu.org/PR100678> "[OpenACC/nvptx]
> 'libgomp.oacc-c-c++-common/private-atomic-1.c' FAILs (differently) in
> certain configurations"...  :-\

..., and that I also confirm to be resolved with
commit e0451f93d9faa13495132f4e246e9bef30b51417
"[nvptx] Add some support for .local atomics".


Grüße
 Thomas
-----------------
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

Re: Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812] (was: [PATCH][testsuite, nvptx] Add effective target sync_int_long_stack)

[Tom de Vries]

On 2/3/22 10:40, Thomas Schwinge wrote:
> Hi Tom!
> 
> On 2021-05-19T14:56:17+0200, I wrote:
>> On 2020-08-12T15:57:23+0200, Tom de Vries <tdevries@suse.de> wrote:
>>> When enabling sync_int_long for nvptx, we run into a failure in
>>> gcc.dg/pr86314.c:
>>> ...
>>>   nvptx-run: error getting kernel result: operation not supported on \
>>>     global/shared address space
>>> ...
>>> due to a ptx restriction:  accesses to local memory are illegal, and the
>>> test-case does an atomic operation on a stack address, which is mapped to
>>> local memory.
>>
>> Now, that problem also is easy to reproduce in an OpenACC/nvptx
>> offloading setting.  (..., as Julian had reported and analyzed in an
>> internal "Date: Fri, 31 May 2019 00:29:17 +0100" email, similar to Tom's
>> comments in PR96494 "[nvptx] Enable effective target sync_int_long" and
>> PR97444 "[nvptx] stack atomics".)
>>
>>> Fix this by adding a target sync_int_long_stack, wich returns false for nvptx,
>>> which can be used to mark test-cases that require sync_int_long support for
>>> stack address.
> 
> Shouldn't this now again be reverted/removed after your
> commit e0451f93d9faa13495132f4e246e9bef30b51417
> "[nvptx] Add some support for .local atomics"?
> (But I haven't looked in detail.)
> 
> 
> Is PR83812 "nvptx-run: error getting kernel result: operation not
> supported on global/shared address space" resolved then?
> 
> Or, because of:
> 
> | This only solves some cases that are detected at compile-time.
> 
> ... not completely resolved?
> 

Yeah, I focused for now just on getting the libgomp testsuite passing.

The generic issue hasn't been fixed yet.  If you take the address of 
stack variable which results in generic addressing, you'll run into the 
same issue.  Fixing that'll involve runtime tests, which probably means 
introducing a -mstack-atomics or some such.

> There's also still PR97444 "[nvptx] stack atomics" open.
> 
> 
>> Similar to PR97444 "[nvptx] stack atomics", such a conditional is of
>> course not applicable for the OpenACC implementation, so to at least
>> document/test/XFAIL nvptx offloading: PR83812 "operation not supported
>> on global/shared address space", I've now pushed to master branch
>> "Add 'libgomp.oacc-c-c++-common/private-atomic-1.c' [PR83812]" in
>> commit 1467100fc72562a59f70cdd4e05f6c810d1fadcc, see attached.
> 
> ... which later got replicated in other test cases --
> all of which I confirm are resolved and un-XFAILed with
> commit e0451f93d9faa13495132f4e246e9bef30b51417
> "[nvptx] Add some support for .local atomics".
> 
> 
>> And then, I got back testresults from one more system, and I've filed
>> <https://gcc.gnu.org/PR100678> "[OpenACC/nvptx]
>> 'libgomp.oacc-c-c++-common/private-atomic-1.c' FAILs (differently) in
>> certain configurations"...  :-\
> 
> ..., and that I also confirm to be resolved with
> commit e0451f93d9faa13495132f4e246e9bef30b51417
> "[nvptx] Add some support for .local atomics".
> 
> 

Ack, thanks for confirming.

Thanks,
- Tom