* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
@ 2020-09-03 13:02 ` vries at gcc dot gnu.org
2020-09-04 8:08 ` vries at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-03 13:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
Tom de Vries <vries at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at redhat dot com
--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, so libatomic needs to fall back onto protect_start and protect_end.
It would make sense for the openmp/openacc programs to have that map onto
GOMP_start/GOMP_stop.
But this introduces a dependency of libatomic on libgomp. Not ideal, I
suppose.
Maybe we could (at least for the nvptx case) move the global lock out of
libgomp into libatomic, and have a dependency of libgomp on libatomic instead.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
2020-09-03 13:02 ` [Bug target/96898] " vries at gcc dot gnu.org
@ 2020-09-04 8:08 ` vries at gcc dot gnu.org
2020-09-04 8:45 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-04 8:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, I found this difference:
- AFAIU, GOMP_atomic_start/end have barrier semantics
- libatomics protect_start/end are always paired with explicit barriers, so
presumably these don't have barrier semantics
So, using GOMP_atomic_start for protect_start in libatomics will have the
effect of issuing the barrier twice, which might be a performance problem.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
2020-09-03 13:02 ` [Bug target/96898] " vries at gcc dot gnu.org
2020-09-04 8:08 ` vries at gcc dot gnu.org
@ 2020-09-04 8:45 ` jakub at gcc dot gnu.org
2020-09-04 16:57 ` vries at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-04 8:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For OpenMP reductions, we really don't care what kind of mutex protects the
updates, as long as it is the same for all updates of the same reduction.
I believe we don't rely on any other synchronization effects.
So, I think we should change omp-low.c so that it emits __atomic_* calls with
__ATOMIC_RELAXED rather than __sync_* calls. And could just use libatomic with
its own locking if we didn't go the GOMP_atomic_{start,end} route (that one is
done if there are multiple reductions or the atomics aren't available or there
are user defined reductions we don't understand (or all?), perhaps we should
consider also using atomics perhaps even for two simple reductions or similar.
And nvptx certainly could just use libatomic...
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (2 preceding siblings ...)
2020-09-04 8:45 ` jakub at gcc dot gnu.org
@ 2020-09-04 16:57 ` vries at gcc dot gnu.org
2020-09-04 17:11 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-04 16:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> For OpenMP reductions, we really don't care what kind of mutex protects the
> updates, as long as it is the same for all updates of the same reduction.
> I believe we don't rely on any other synchronization effects.
> So, I think we should change omp-low.c so that it emits __atomic_* calls
> with __ATOMIC_RELAXED rather than __sync_* calls.
That sounds like a good idea.
> And could just use
> libatomic with its own locking if we didn't go the GOMP_atomic_{start,end}
> route (that one is done if there are multiple reductions or the atomics
> aren't available or there are user defined reductions we don't understand
> (or all?), perhaps we should consider also using atomics perhaps even for
> two simple reductions or similar.
> And nvptx certainly could just use libatomic...
If we use libatomic as fallback for openmp, shouldn't we then use the same lock
in both?
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (3 preceding siblings ...)
2020-09-04 16:57 ` vries at gcc dot gnu.org
@ 2020-09-04 17:11 ` jakub at gcc dot gnu.org
2020-09-07 22:17 ` vries at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-04 17:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It wouldn't be a fallback. omp-low.c just decides if it is going to use
GOMP_atomic_{start,end} synchronization, __atomic_* or __sync_* to perform the
reduction. And whether that uses the same or different lock doesn't matter,
because for one reduction omp-low.c will only use one way.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (4 preceding siblings ...)
2020-09-04 17:11 ` jakub at gcc dot gnu.org
@ 2020-09-07 22:17 ` vries at gcc dot gnu.org
2020-09-07 22:50 ` vries at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-07 22:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #6 from Tom de Vries <vries at gcc dot gnu.org> ---
Created attachment 49195
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49195&action=edit
Tentative patch
Introduces an option -fatomic-libcalls (analogous to -fsync-libcalls) such that
__atomic_test_and_set maps onto libatomic function __atomic_test_and_set_1.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (5 preceding siblings ...)
2020-09-07 22:17 ` vries at gcc dot gnu.org
@ 2020-09-07 22:50 ` vries at gcc dot gnu.org
2020-09-08 6:52 ` vries at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-07 22:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #7 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom de Vries from comment #6)
> Created attachment 49195 [details]
> Tentative patch
>
> Introduces an option -fatomic-libcalls (analogous to -fsync-libcalls) such
> that __atomic_test_and_set maps onto libatomic function
> __atomic_test_and_set_1.
I've now achieved the same in the target, not relying on a new option:
...
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index 4168190fa42..6178e6a0f77 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -1667,6 +1667,22 @@
"%.\\tatom%A1.b%T0.<logic>\\t%0, %1, %2;"
[(set_attr "atomic" "true")])
+(define_expand "atomic_test_and_set"
+ [(match_operand:SI 0 "nvptx_register_operand") ;; output
+ (match_operand:QI 1 "memory_operand") ;; memory
+ (match_operand:SI 2 "const_int_operand")] ;; model
+ ""
+{
+ rtx libfunc;
+ rtx addr;
+ libfunc = init_one_libfunc ("__atomic_test_and_set_1");
+ addr = convert_memory_address (ptr_mode, XEXP (operands[1], 0));
+ emit_library_call_value (libfunc, operands[0], LCT_NORMAL, SImode,
+ addr, ptr_mode,
+ operands[2], SImode);
+ DONE;
+})
+
(define_insn "nvptx_barsync"
[(unspec_volatile [(match_operand:SI 0 "nvptx_nonmemory_operand" "Ri")
(match_operand:SI 1 "const_int_operand")]
...
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (6 preceding siblings ...)
2020-09-07 22:50 ` vries at gcc dot gnu.org
@ 2020-09-08 6:52 ` vries at gcc dot gnu.org
2020-09-11 10:08 ` cvs-commit at gcc dot gnu.org
2020-09-11 10:18 ` vries at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-08 6:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
Patch submitted:
https://gcc.gnu.org/pipermail/gcc-patches/2020-September/553393.html
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (7 preceding siblings ...)
2020-09-08 6:52 ` vries at gcc dot gnu.org
@ 2020-09-11 10:08 ` cvs-commit at gcc dot gnu.org
2020-09-11 10:18 ` vries at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-09-11 10:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@gcc.gnu.org>:
https://gcc.gnu.org/g:15545563128f0240192c263522d4a36b7f86250f
commit r11-3145-g15545563128f0240192c263522d4a36b7f86250f
Author: Tom de Vries <tdevries@suse.de>
Date: Mon Sep 7 10:47:25 2020 +0200
[libatomic] Add nvptx support
Add nvptx support to libatomic.
Given that atomic_test_and_set is not implemented for nvptx (PR96964), the
compiler translates __atomic_test_and_set falling back onto the "Failing
all
else, assume a single threaded environment and simply perform the
operation"
case in expand_atomic_test_and_set, so it doesn't map onto an actual atomic
operation.
Still, that counts as supported for the configure test of libatomic, so we
end up with HAVE_ATOMIC_TAS_1/2/4/8/16 == 1, and the corresponding
__atomic_test_and_set_1/2/4/8/16 in libatomic all using that non-atomic
implementation.
Fix this by adding an atomic_test_and_set expansion for nvptx, that uses
libatomics __atomic_test_and_set_1.
This again makes the configure tests for HAVE_ATOMIC_TAS_1/2/4/8/16 fail,
so
instead we use this case in tas_n.c:
...
/* If this type is smaller than word-sized, fall back to a word-sized
compare-and-swap loop. */
bool
SIZE(libat_test_and_set) (UTYPE *mptr, int smodel)
...
which for __atomic_test_and_set_8 uses INVERT_MASK_8.
Add INVERT_MASK_8 in libatomic_i.h, as well as MASK_8.
Tested libatomic testsuite on nvptx.
gcc/ChangeLog:
PR target/96964
* config/nvptx/nvptx.md (define_expand "atomic_test_and_set"): New
expansion.
libatomic/ChangeLog:
PR target/96898
* configure.tgt: Add nvptx.
* libatomic_i.h (MASK_8, INVERT_MASK_8): New macro definition.
* config/nvptx/host-config.h: New file.
* config/nvptx/lock.c: New file.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug target/96898] [nvptx] libatomic support
2020-09-02 11:41 [Bug target/96898] New: [nvptx] libatomic support vries at gcc dot gnu.org
` (8 preceding siblings ...)
2020-09-11 10:08 ` cvs-commit at gcc dot gnu.org
@ 2020-09-11 10:18 ` vries at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: vries at gcc dot gnu.org @ 2020-09-11 10:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96898
Tom de Vries <vries at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
Target Milestone|--- |11.0
--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
Patch committed, marking resolved-fixed.
^ permalink raw reply [flat|nested] 11+ messages in thread