On Thu, Apr 24, 2025 at 12:06:52PM -0400, Jason Merrill wrote:
> On 4/22/25 4:48 PM, Jason Merrill wrote:
> > On 4/22/25 1:21 PM, Tobias Burnus wrote:
> > > Jason Merrill wrote:
> > > > On 4/22/25 11:04 AM, Tobias Burnus wrote:
> > > > > The question is why does this code trigger at all, given
> > > > > that there is OpenMP but no offload code at all? And how
> > > > > to fix it in case there is offload code and modules are used.
> > > >
> > > > This seems to be because of:
> > > >
> > > > > if (module_global_init_needed ())
> > > > > {
> > > > > // Make sure there's a default priority entry.
> > > > > if (! static_init_fini_fns[true])
> > > > > static_init_fini_fns[true] = priority_map_t::create_ggc ();
> > > > > if (static_init_fini_fns[true]->get_or_insert
> > > > > (DEFAULT_INIT_PRIORITY))
> > > > > has_module_inits = true;
> > > > >
> > > > > if (flag_openmp)
> > > > > {
> > > > > if (!static_init_fini_fns[2 + true])
> > > > > static_init_fini_fns[2 + true] =
> > > > > priority_map_t::create_ggc ();
> > > > > static_init_fini_fns[2 + true]->get_or_insert
> > > > > (DEFAULT_INIT_PRIORITY);
> > > > > }
> > > > > }
> > > >
> > > > Here we're forcing a target module init function as well as
> > > > host. If we remove the flag_openmp block, Nathaniel's patch is
> > > > unnecessary (but may still be desirable).
> > >
> > > I currently do not see whether the code is needed in this case or
> > > not, but I assume it is, if we want to support static
> > > initializers?!?
> >
> > I don't think so. For the host, we force create a map with a single
> > entry because we always want to emit a module init function. The openmp
> > block is saying we also always want a target init function in a module,
> > even if it's empty, which I don't think is correct. Or if it is, we
> > need to specify how to mangle it and agree that that's part of the
> > module ABI.
>
> So, for now in addition to Nathaniel's patch I'd remove this flag_openmp
> block so we don't get an unneeded empty function, and maybe add something
> back later after more discussion.
>
> Jason
>
So something like this, perhaps?
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
-- >8 --
In r15-2799-gf1bfba3a9b3f31, a new kind of global constructor was added.
Unfortunately this broke C++20 modules, as both the host and target
constructors were given the same mangled name. This patch ensures that
only the host constructor gets the module name mangling for now, and
stops forcing the creation of the target constructor even when no such
initialization is required.
PR c++/119864
gcc/cp/ChangeLog:
* decl2.cc (start_objects): Only use module initialized for
host.
(c_parse_final_cleanups): Don't always create an OMP offload
init function in modules.
gcc/testsuite/ChangeLog:
* g++.dg/modules/openmp-1.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/decl2.cc | 14 +++++++-------
gcc/testsuite/g++.dg/modules/openmp-1.C | 9 +++++++++
2 files changed, 16 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/openmp-1.C
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 21156f1dd3b..d29d93af275 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -4184,7 +4184,11 @@ start_objects (bool initp, unsigned priority, bool
has_body,
bool omp_target = false)
{
bool default_init = initp && priority == DEFAULT_INIT_PRIORITY;
- bool is_module_init = default_init && module_global_init_needed ();
+ /* FIXME: We may eventually want to treat OpenMP offload initialisers
+ in modules specially as well. */
+ bool is_module_init = (default_init
+ && !omp_target
+ && module_global_init_needed ());
tree name = NULL_TREE;
if (is_module_init)
@@ -5876,12 +5880,8 @@ c_parse_final_cleanups (void)
if (static_init_fini_fns[true]->get_or_insert (DEFAULT_INIT_PRIORITY))
has_module_inits = true;
- if (flag_openmp)
- {
- if (!static_init_fini_fns[2 + true])
- static_init_fini_fns[2 + true] = priority_map_t::create_ggc ();
- static_init_fini_fns[2 + true]->get_or_insert (DEFAULT_INIT_PRIORITY);
- }
+ /* FIXME: We need to work out what static constructors on OpenMP offload
+ target in modules will look like. */
}
/* Generate initialization and destruction functions for all
diff --git a/gcc/testsuite/g++.dg/modules/openmp-1.C
b/gcc/testsuite/g++.dg/modules/openmp-1.C
new file mode 100644
index 00000000000..b5a30ad8c91
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/openmp-1.C
@@ -0,0 +1,9 @@
+// PR c++/119864
+// { dg-do assemble }
+// { dg-additional-options "-fmodules -fopenmp" }
+// { dg-require-effective-target "fopenmp" }
+
+export module M;
+
+int foo();
+int x = foo();
--
2.47.0
