Hi Jakub! On 2023-12-07T16:33:08+0100, Jakub Jelinek <ja...@redhat.com> 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 <tho...@codesourcery.com> 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