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

Reply via email to