Hi Jakub!
On 2023-12-07T16:33:08+0100, Jakub Jelinek <[email protected]> wrote:
> On Thu, Dec 07, 2023 at 04:09:04PM +0100, Thomas Schwinge wrote:
>> > Yeah, I believe we should in the omp_discover_* sub-pass handle with
>> > a help of a langhook automatically mark the guard variables (possibly
>> > iff the guarded variable is marked?),
>>
>> Looking at 'gcc/omp-offload.cc:omp_discover_implicit_declare_target' left
>> me confused how that would be the code that marks up 'static' variables
>> as implicit 'omp declare target'. Working through a simple POD example
>> (say, 's%static S s%static int i') it turns out, indeed that's not where
>> that is happending, but instead 'gcc/gimplify.cc:gimplify_bind_expr' is
>> the place:
>
> Sure, that is for the case where those local statics should be marked
> implicitly because they appear in a target function.
> They can be also marked explicitly by the user through
> #pragma omp declare target enter (name_of_static_var)
> or
> [[omp::decl (declare target)]] attribute on it etc.
These three: implicitly, or explicit '#pragma omp declare target' etc.,
or inside '#pragma omp begin declare target' region are the only OpenMP
facilities to get things 'omp declare target'ed, right?
>> That said... Couldn't we indeed move this gimplification-level code re
>> 'Static locals [...] need to be "omp declare target"' into
>> 'gcc/omp-offload.cc:omp_discover_implicit_declare_target'?
>
> The omp-offload.cc discovery stuff was added for stuff where the OpenMP
> standard says something is implicitly declare target because there is
> some use of it satisfying some rule.
> Like, calls to functions defined in current compilation unit referenced in
> target region or something similar, or such calls referenced in declare
> target static var initializers.
> So, that feels to me like the right spot to handle the guards as well.
> Of course, the middle-end doesn't know about C++ FE's get_guard variable,
> so it should be some new language hook which would take care of it.
> The omp_discover_declare* functions can add further VAR_DECLs to the
> worklist, so I'd probably call the new language hook in the
> omp_discover_implicit_declare_target last loop.
> Or maybe even better just handle that in the
> cxx_omp_finish_decl_inits hook. You can just
> FOR_EACH_VARIABLE (vnode)
> if (DECL_FUNCTION_SCOPE_P (vnode->decl)
> && omp_declare_target_var_p (vnode->decl))
> {
> tree sname = mangle_guard_variable (decl);
> tree guard = get_global_binding (sname);
> if (guard)
> ... mark guard as declare target if not yet marked ...
> }
> because guard var initializers don't really mention anything and so
> their addition doesn't need to trigger further worklist changes.
That doesn't generally work, as the gimplification-level code re
'Static locals [...] need to be "omp declare target"' runs *after*
'omp_discover_implicit_declare_target'. Thus my "move" idea above.
However, let's defer the latter one; I've now got a simple setup where
the new language hook is invoked in all necessary places. (Will post
later.)
>> > And sure, __cxa_guard_* would need to be implemented in the offloading
>> > libsupc++.a or libstdc++.a.
>>
>> Until proper libstdc++/libsupc++ support emerges (I'm working on it...),
>> my idea was to add a temporary 'libgomp/config/accel/*.c' implementation
>> (based on 'libstdc++-v3/libsupc++/guard.cc').
>
> That looks reasonable.
OK to push, for a start, the attached
"GCN, nvptx: Basic '__cxa_guard_{acquire,abort,release}' for C++ static local
variables support"?
That's now in libgcc not libgomp, so that it's also usable for GCN, nvptx
target testing, where we thus see a number of FAIL -> PASS progressions.
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
>From d40678768ae90c3fe1208cffd7d92e7058db5bbf Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <[email protected]>
Date: Wed, 20 Dec 2023 12:27:48 +0100
Subject: [PATCH] GCN, nvptx: Basic '__cxa_guard_{acquire,abort,release}' for
C++ static local variables support
For now, for single-threaded GCN, nvptx target use only; extension for
multi-threaded offloading use to follow later.
libgcc/
* c++-minimal/README: New.
* c++-minimal/guard.c: New.
* config/gcn/t-amdgcn (LIB2ADD): Add it.
* config/nvptx/t-nvptx (LIB2ADD): Likewise.
---
libgcc/c++-minimal/README | 2 +
libgcc/c++-minimal/guard.c | 97 +++++++++++++++++++++++++++++++++++++
libgcc/config/gcn/t-amdgcn | 3 ++
libgcc/config/nvptx/t-nvptx | 3 ++
4 files changed, 105 insertions(+)
create mode 100644 libgcc/c++-minimal/README
create mode 100644 libgcc/c++-minimal/guard.c
diff --git a/libgcc/c++-minimal/README b/libgcc/c++-minimal/README
new file mode 100644
index 00000000000..832f1265f7e
--- /dev/null
+++ b/libgcc/c++-minimal/README
@@ -0,0 +1,2 @@
+Minimal hacked-up version of some C++ support for offload devices, until we
+have libstdc++-v3/libsupc++ proper.
diff --git a/libgcc/c++-minimal/guard.c b/libgcc/c++-minimal/guard.c
new file mode 100644
index 00000000000..9f0a70af2ce
--- /dev/null
+++ b/libgcc/c++-minimal/guard.c
@@ -0,0 +1,97 @@
+/* 'libstdc++-v3/libsupc++/guard.cc' for offload devices, until we have
+ libstdc++-v3/libsupc++ proper.
+
+ Copyright (C) 2002-2023 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+<http://www.gnu.org/licenses/>. */
+
+#if defined __AMDGCN__
+#elif defined __nvptx__
+#else
+# error not ported
+#endif
+
+#include "../../libstdc++-v3/config/cpu/generic/cxxabi_tweaks.h"
+
+/* Copy'n'paste/edit from 'libstdc++-v3/libsupc++/cxxabi.h'. */
+
+ int
+ __cxa_guard_acquire(__guard*);
+
+ void
+ __cxa_guard_release(__guard*);
+
+ void
+ __cxa_guard_abort(__guard*);
+
+/* Copy'n'paste/edit from 'libstdc++-v3/libsupc++/guard.cc'. */
+
+# undef _GLIBCXX_GUARD_TEST_AND_ACQUIRE
+# undef _GLIBCXX_GUARD_SET_AND_RELEASE
+# define _GLIBCXX_GUARD_SET_AND_RELEASE(G) _GLIBCXX_GUARD_SET (G)
+
+ static inline int
+ init_in_progress_flag(__guard* g)
+ { return ((char *)g)[1]; }
+
+ static inline void
+ set_init_in_progress_flag(__guard* g, int v)
+ { ((char *)g)[1] = v; }
+
+ static inline void
+ throw_recursive_init_exception(void)
+ {
+ // Use __builtin_trap so we don't require abort().
+ __builtin_trap();
+ }
+
+ // acquire() is a helper function used to acquire guard if thread support is
+ // not compiled in or is compiled in but not enabled at run-time.
+ static int
+ acquire(__guard *g)
+ {
+ // Quit if the object is already initialized.
+ if (_GLIBCXX_GUARD_TEST(g))
+ return 0;
+
+ if (init_in_progress_flag(g))
+ throw_recursive_init_exception();
+
+ set_init_in_progress_flag(g, 1);
+ return 1;
+ }
+
+ int __cxa_guard_acquire (__guard *g)
+ {
+ return acquire (g);
+ }
+
+ void __cxa_guard_abort (__guard *g)
+ {
+ set_init_in_progress_flag(g, 0);
+ }
+
+ void __cxa_guard_release (__guard *g)
+ {
+ set_init_in_progress_flag(g, 0);
+ _GLIBCXX_GUARD_SET_AND_RELEASE (g);
+ }
diff --git a/libgcc/config/gcn/t-amdgcn b/libgcc/config/gcn/t-amdgcn
index d1d9a4f92b5..b00adc72bad 100644
--- a/libgcc/config/gcn/t-amdgcn
+++ b/libgcc/config/gcn/t-amdgcn
@@ -8,6 +8,9 @@ LIB2ADD += $(srcdir)/config/gcn/atomic.c \
$(srcdir)/config/gcn/lib2-bswapti2.c \
$(srcdir)/config/gcn/unwind-gcn.c
+# Until we have libstdc++-v3/libsupc++ proper.
+LIB2ADD += $(srcdir)/c++-minimal/guard.c
+
LIB2ADDEH=
LIB2FUNCS_EXCLUDE=__main
diff --git a/libgcc/config/nvptx/t-nvptx b/libgcc/config/nvptx/t-nvptx
index ede0bf0f87d..49fdb557b56 100644
--- a/libgcc/config/nvptx/t-nvptx
+++ b/libgcc/config/nvptx/t-nvptx
@@ -2,6 +2,9 @@ LIB2ADD=$(srcdir)/config/nvptx/reduction.c \
$(srcdir)/config/nvptx/mgomp.c \
$(srcdir)/config/nvptx/atomic.c
+# Until we have libstdc++-v3/libsupc++ proper.
+LIB2ADD += $(srcdir)/c++-minimal/guard.c
+
LIB2ADDEH=
LIB2FUNCS_EXCLUDE=__main
--
2.34.1
