public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++/modules: Finalise non-local imported vars [PR113708]
@ 2024-02-11 13:26 Nathaniel Shead
  2024-02-13 23:08 ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Nathaniel Shead @ 2024-02-11 13:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell, Patrick Palka

Bootstrapped and regtested (just modules.exp so far) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?

-- >8 --

Currently inline vars imported from modules aren't correctly finalised,
which means that import_export_decl gets called at the end of TU
processing despite not being meant to for these kinds of declarations.

This patch takes the logic from 'make_rtl_for_nonlocal_decl' to
determine when to perform rest of decl compilation on these decls.
However other parts of this function (asmspec handling) are not yet
covered.

	PR c++/113708

gcc/cp/ChangeLog:

	* module.cc (trees_in::read_var_def): Perform rest of decl
	compilation on non-local statics.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/init-7_a.H: New test.
	* g++.dg/modules/init-7_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                        | 12 +++++++++++-
 gcc/testsuite/g++.dg/modules/init-7_a.H |  6 ++++++
 gcc/testsuite/g++.dg/modules/init-7_b.C |  6 ++++++
 3 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 560d8f3b614..49a3421e6cc 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11809,6 +11809,7 @@ trees_in::read_var_def (tree decl, tree maybe_template)
     {
       if (DECL_EXTERNAL (decl))
 	DECL_NOT_REALLY_EXTERN (decl) = true;
+      DECL_INITIAL (decl) = init;
       if (VAR_P (decl))
 	{
 	  DECL_INITIALIZED_P (decl) = true;
@@ -11819,8 +11820,17 @@ trees_in::read_var_def (tree decl, tree maybe_template)
 		  && !DECL_VTABLE_OR_VTT_P (decl)
 		  && !DECL_TEMPLATE_INFO (decl)))
 	    note_vague_linkage_variable (decl);
+
+	  /* Emit RTL as needed, as with make_rtl_for_nonlocal_decl.  */
+	  // FIXME: Handle asmspec?
+	  if ((!DECL_FUNCTION_SCOPE_P (decl)
+	       || (TREE_STATIC (decl) && var_in_maybe_constexpr_fn (decl)))
+	      && !DECL_VIRTUAL_P (decl)
+	      && (!DECL_TEMPLATE_INFO (decl)
+		  || DECL_EXPLICIT_INSTANTIATION (decl)))
+	    rest_of_decl_compilation (decl, !DECL_FUNCTION_SCOPE_P (decl),
+				      /*at_end=*/0);
 	}
-      DECL_INITIAL (decl) = init;
       if (!dyn_init)
 	;
       else if (CP_DECL_THREAD_LOCAL_P (decl))
diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H
new file mode 100644
index 00000000000..7a0bb721c30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_a.H
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+inline int f() { return 42; }
+inline int a = f();
diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C
new file mode 100644
index 00000000000..58bb0620ca5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_b.C
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import "init-7_a.H";
+int main() { a; }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++/modules: Finalise non-local imported vars [PR113708]
  2024-02-11 13:26 [PATCH] c++/modules: Finalise non-local imported vars [PR113708] Nathaniel Shead
@ 2024-02-13 23:08 ` Jason Merrill
  2024-02-14  1:34   ` [PATCH v2] c++: Defer emitting inline variables [PR113708] Nathaniel Shead
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2024-02-13 23:08 UTC (permalink / raw)
  To: Nathaniel Shead, gcc-patches; +Cc: Nathan Sidwell, Patrick Palka

On 2/11/24 08:26, Nathaniel Shead wrote:
> 
> Currently inline vars imported from modules aren't correctly finalised,
> which means that import_export_decl gets called at the end of TU
> processing despite not being meant to for these kinds of declarations.

I disagree that it's not meant to; inline variables are vague linkage 
just like template instantiations, so the bug seems to be that 
import_export_decl doesn't accept them.  And on the other side, that 
make_rtl_for_nonlocal_decl doesn't defer them like instantations.

Jason


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] c++: Defer emitting inline variables [PR113708]
  2024-02-13 23:08 ` Jason Merrill
@ 2024-02-14  1:34   ` Nathaniel Shead
  2024-02-14  2:47     ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Nathaniel Shead @ 2024-02-14  1:34 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell, Patrick Palka

On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote:
> On 2/11/24 08:26, Nathaniel Shead wrote:
> > 
> > Currently inline vars imported from modules aren't correctly finalised,
> > which means that import_export_decl gets called at the end of TU
> > processing despite not being meant to for these kinds of declarations.
> 
> I disagree that it's not meant to; inline variables are vague linkage just
> like template instantiations, so the bug seems to be that import_export_decl
> doesn't accept them.  And on the other side, that make_rtl_for_nonlocal_decl
> doesn't defer them like instantations.
> 
> Jason
> 

True, that's a good point. I think I confused myself here.

Here's a fixed patch that looks a lot cleaner. Bootstrapped and
regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu,
OK for trunk if full regtest succeeds?

-- >8 --

Inline variables are vague-linkage, and may or may not need to be
emitted in any TU that they are part of, similarly to e.g. template
instantiations.

Currently 'import_export_decl' assumes that inline variables have
already been emitted when it comes to end-of-TU processing, and so
crashes when importing non-trivially-initialised variables from a
module, as they have not yet been finalised.

This patch fixes this by ensuring that inline variables are always
deferred till end-of-TU processing, unifying the behaviour for module
and non-module code.

	PR c++/113708

gcc/cp/ChangeLog:

	* decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables.
	* decl2.cc (import_export_decl): Support inline variables.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/init-7_a.H: New test.
	* g++.dg/modules/init-7_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/decl.cc                          | 4 ++++
 gcc/cp/decl2.cc                         | 7 +++++--
 gcc/testsuite/g++.dg/modules/init-7_a.H | 6 ++++++
 gcc/testsuite/g++.dg/modules/init-7_b.C | 6 ++++++
 4 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 3e41fd4fa31..969513c069a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec)
       && DECL_IMPLICIT_INSTANTIATION (decl))
     defer_p = 1;
 
+  /* Defer vague-linkage variables.  */
+  if (DECL_INLINE_VAR_P (decl))
+    defer_p = 1;
+
   /* If we're not deferring, go ahead and assemble the variable.  */
   if (!defer_p)
     rest_of_decl_compilation (decl, toplev, at_eof);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index f569d4045ec..1dddbaab38b 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -3360,7 +3360,9 @@ import_export_decl (tree decl)
 
      * implicit instantiations of function templates
 
-     * inline function
+     * inline functions
+
+     * inline variables
 
      * implicit instantiations of static data members of class
        templates
@@ -3383,6 +3385,7 @@ import_export_decl (tree decl)
 		|| DECL_DECLARED_INLINE_P (decl));
   else
     gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl)
+		|| DECL_INLINE_VAR_P (decl)
 		|| DECL_VTABLE_OR_VTT_P (decl)
 		|| DECL_TINFO_P (decl));
   /* Check that a definition of DECL is available in this translation
@@ -3511,7 +3514,7 @@ import_export_decl (tree decl)
 	   this entity as undefined in this translation unit.  */
 	import_p = true;
     }
-  else if (DECL_FUNCTION_MEMBER_P (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl))
     {
       if (!DECL_DECLARED_INLINE_P (decl))
 	{
diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H
new file mode 100644
index 00000000000..7a0bb721c30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_a.H
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+inline int f() { return 42; }
+inline int a = f();
diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C
new file mode 100644
index 00000000000..58bb0620ca5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_b.C
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import "init-7_a.H";
+int main() { a; }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
  2024-02-14  1:34   ` [PATCH v2] c++: Defer emitting inline variables [PR113708] Nathaniel Shead
@ 2024-02-14  2:47     ` Jason Merrill
  2024-02-14 11:03       ` Nathaniel Shead
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2024-02-14  2:47 UTC (permalink / raw)
  To: Nathaniel Shead; +Cc: gcc-patches, Nathan Sidwell, Patrick Palka

On 2/13/24 20:34, Nathaniel Shead wrote:
> On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote:
>> On 2/11/24 08:26, Nathaniel Shead wrote:
>>>
>>> Currently inline vars imported from modules aren't correctly finalised,
>>> which means that import_export_decl gets called at the end of TU
>>> processing despite not being meant to for these kinds of declarations.
>>
>> I disagree that it's not meant to; inline variables are vague linkage just
>> like template instantiations, so the bug seems to be that import_export_decl
>> doesn't accept them.  And on the other side, that make_rtl_for_nonlocal_decl
>> doesn't defer them like instantations.
>>
>> Jason
>>
> 
> True, that's a good point. I think I confused myself here.
> 
> Here's a fixed patch that looks a lot cleaner. Bootstrapped and
> regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu,
> OK for trunk if full regtest succeeds?

OK.

> -- >8 --
> 
> Inline variables are vague-linkage, and may or may not need to be
> emitted in any TU that they are part of, similarly to e.g. template
> instantiations.
> 
> Currently 'import_export_decl' assumes that inline variables have
> already been emitted when it comes to end-of-TU processing, and so
> crashes when importing non-trivially-initialised variables from a
> module, as they have not yet been finalised.
> 
> This patch fixes this by ensuring that inline variables are always
> deferred till end-of-TU processing, unifying the behaviour for module
> and non-module code.
> 
> 	PR c++/113708
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables.
> 	* decl2.cc (import_export_decl): Support inline variables.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/init-7_a.H: New test.
> 	* g++.dg/modules/init-7_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/decl.cc                          | 4 ++++
>   gcc/cp/decl2.cc                         | 7 +++++--
>   gcc/testsuite/g++.dg/modules/init-7_a.H | 6 ++++++
>   gcc/testsuite/g++.dg/modules/init-7_b.C | 6 ++++++
>   4 files changed, 21 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 3e41fd4fa31..969513c069a 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec)
>         && DECL_IMPLICIT_INSTANTIATION (decl))
>       defer_p = 1;
>   
> +  /* Defer vague-linkage variables.  */
> +  if (DECL_INLINE_VAR_P (decl))
> +    defer_p = 1;
> +
>     /* If we're not deferring, go ahead and assemble the variable.  */
>     if (!defer_p)
>       rest_of_decl_compilation (decl, toplev, at_eof);
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index f569d4045ec..1dddbaab38b 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -3360,7 +3360,9 @@ import_export_decl (tree decl)
>   
>        * implicit instantiations of function templates
>   
> -     * inline function
> +     * inline functions
> +
> +     * inline variables
>   
>        * implicit instantiations of static data members of class
>          templates
> @@ -3383,6 +3385,7 @@ import_export_decl (tree decl)
>   		|| DECL_DECLARED_INLINE_P (decl));
>     else
>       gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl)
> +		|| DECL_INLINE_VAR_P (decl)
>   		|| DECL_VTABLE_OR_VTT_P (decl)
>   		|| DECL_TINFO_P (decl));
>     /* Check that a definition of DECL is available in this translation
> @@ -3511,7 +3514,7 @@ import_export_decl (tree decl)
>   	   this entity as undefined in this translation unit.  */
>   	import_p = true;
>       }
> -  else if (DECL_FUNCTION_MEMBER_P (decl))
> +  else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl))
>       {
>         if (!DECL_DECLARED_INLINE_P (decl))
>   	{
> diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H
> new file mode 100644
> index 00000000000..7a0bb721c30
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/init-7_a.H
> @@ -0,0 +1,6 @@
> +// PR c++/113708
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +inline int f() { return 42; }
> +inline int a = f();
> diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C
> new file mode 100644
> index 00000000000..58bb0620ca5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/init-7_b.C
> @@ -0,0 +1,6 @@
> +// PR c++/113708
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "init-7_a.H";
> +int main() { a; }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
  2024-02-14  2:47     ` Jason Merrill
@ 2024-02-14 11:03       ` Nathaniel Shead
  2024-02-14 13:34         ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Nathaniel Shead @ 2024-02-14 11:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell, Patrick Palka

On Tue, Feb 13, 2024 at 09:47:27PM -0500, Jason Merrill wrote:
> On 2/13/24 20:34, Nathaniel Shead wrote:
> > On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote:
> > > On 2/11/24 08:26, Nathaniel Shead wrote:
> > > > 
> > > > Currently inline vars imported from modules aren't correctly finalised,
> > > > which means that import_export_decl gets called at the end of TU
> > > > processing despite not being meant to for these kinds of declarations.
> > > 
> > > I disagree that it's not meant to; inline variables are vague linkage just
> > > like template instantiations, so the bug seems to be that import_export_decl
> > > doesn't accept them.  And on the other side, that make_rtl_for_nonlocal_decl
> > > doesn't defer them like instantations.
> > > 
> > > Jason
> > > 
> > 
> > True, that's a good point. I think I confused myself here.
> > 
> > Here's a fixed patch that looks a lot cleaner. Bootstrapped and
> > regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu,
> > OK for trunk if full regtest succeeds?
> 
> OK.
> 

A full bootstrap failed two tests in dwarf2.exp, which seem to be caused
by an unreferenced 'inline' variable not being emitted into the debug
info and thus causing the checks for its existence to fail. Adding a
reference to the vars cause the tests to pass.

Now fully bootstrapped and regtested on x86_64-pc-linux-gnu, still OK
for trunk? (Only change is the two adjusted testcases.)

-- >8 --

Inline variables are vague-linkage, and may or may not need to be
emitted in any TU that they are part of, similarly to e.g. template
instantiations.

Currently 'import_export_decl' assumes that inline variables have
already been emitted when it comes to end-of-TU processing, and so
crashes when importing non-trivially-initialised variables from a
module, as they have not yet been finalised.

This patch fixes this by ensuring that inline variables are always
deferred till end-of-TU processing, unifying the behaviour for module
and non-module code.

	PR c++/113708

gcc/cp/ChangeLog:

	* decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables.
	* decl2.cc (import_export_decl): Support inline variables.

gcc/testsuite/ChangeLog:

	* g++.dg/debug/dwarf2/inline-var-1.C: Reference 'a' to ensure it
	is emitted.
	* g++.dg/debug/dwarf2/inline-var-3.C: Likewise.
	* g++.dg/modules/init-7_a.H: New test.
	* g++.dg/modules/init-7_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/decl.cc                                   | 4 ++++
 gcc/cp/decl2.cc                                  | 7 +++++--
 gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C | 2 ++
 gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C | 2 ++
 gcc/testsuite/g++.dg/modules/init-7_a.H          | 6 ++++++
 gcc/testsuite/g++.dg/modules/init-7_b.C          | 6 ++++++
 6 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 3e41fd4fa31..969513c069a 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec)
       && DECL_IMPLICIT_INSTANTIATION (decl))
     defer_p = 1;
 
+  /* Defer vague-linkage variables.  */
+  if (DECL_INLINE_VAR_P (decl))
+    defer_p = 1;
+
   /* If we're not deferring, go ahead and assemble the variable.  */
   if (!defer_p)
     rest_of_decl_compilation (decl, toplev, at_eof);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index f569d4045ec..1dddbaab38b 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -3360,7 +3360,9 @@ import_export_decl (tree decl)
 
      * implicit instantiations of function templates
 
-     * inline function
+     * inline functions
+
+     * inline variables
 
      * implicit instantiations of static data members of class
        templates
@@ -3383,6 +3385,7 @@ import_export_decl (tree decl)
 		|| DECL_DECLARED_INLINE_P (decl));
   else
     gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl)
+		|| DECL_INLINE_VAR_P (decl)
 		|| DECL_VTABLE_OR_VTT_P (decl)
 		|| DECL_TINFO_P (decl));
   /* Check that a definition of DECL is available in this translation
@@ -3511,7 +3514,7 @@ import_export_decl (tree decl)
 	   this entity as undefined in this translation unit.  */
 	import_p = true;
     }
-  else if (DECL_FUNCTION_MEMBER_P (decl))
+  else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl))
     {
       if (!DECL_DECLARED_INLINE_P (decl))
 	{
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
index 85f74a91521..7ec20afc065 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
@@ -8,6 +8,8 @@
 // { dg-final { scan-assembler-times " DW_AT_\[^\n\r]*linkage_name" 7 } }
 
 inline int a;
+int& ar = a;
+
 struct S
 {
   static inline double b = 4.0;
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
index 6345b5e2ec8..bb40a229551 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
@@ -11,6 +11,8 @@
 // { dg-final { scan-assembler-times " DW_AT_\[^\n\r]*linkage_name" 7 } }
 
 inline int a;
+int& ar = a;
+
 struct S
 {
   static inline double b = 4.0;
diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H
new file mode 100644
index 00000000000..7a0bb721c30
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_a.H
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+inline int f() { return 42; }
+inline int a = f();
diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C
new file mode 100644
index 00000000000..58bb0620ca5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/init-7_b.C
@@ -0,0 +1,6 @@
+// PR c++/113708
+// { dg-module-do link }
+// { dg-additional-options "-fmodules-ts" }
+
+import "init-7_a.H";
+int main() { a; }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
  2024-02-14 11:03       ` Nathaniel Shead
@ 2024-02-14 13:34         ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2024-02-14 13:34 UTC (permalink / raw)
  To: Nathaniel Shead; +Cc: gcc-patches, Nathan Sidwell, Patrick Palka

On 2/14/24 06:03, Nathaniel Shead wrote:
> On Tue, Feb 13, 2024 at 09:47:27PM -0500, Jason Merrill wrote:
>> On 2/13/24 20:34, Nathaniel Shead wrote:
>>> On Tue, Feb 13, 2024 at 06:08:42PM -0500, Jason Merrill wrote:
>>>> On 2/11/24 08:26, Nathaniel Shead wrote:
>>>>>
>>>>> Currently inline vars imported from modules aren't correctly finalised,
>>>>> which means that import_export_decl gets called at the end of TU
>>>>> processing despite not being meant to for these kinds of declarations.
>>>>
>>>> I disagree that it's not meant to; inline variables are vague linkage just
>>>> like template instantiations, so the bug seems to be that import_export_decl
>>>> doesn't accept them.  And on the other side, that make_rtl_for_nonlocal_decl
>>>> doesn't defer them like instantations.
>>>>
>>>> Jason
>>>>
>>>
>>> True, that's a good point. I think I confused myself here.
>>>
>>> Here's a fixed patch that looks a lot cleaner. Bootstrapped and
>>> regtested (so far just dg.exp and modules.exp) on x86_64-pc-linux-gnu,
>>> OK for trunk if full regtest succeeds?
>>
>> OK.
>>
> 
> A full bootstrap failed two tests in dwarf2.exp, which seem to be caused
> by an unreferenced 'inline' variable not being emitted into the debug
> info and thus causing the checks for its existence to fail. Adding a
> reference to the vars cause the tests to pass.
> 
> Now fully bootstrapped and regtested on x86_64-pc-linux-gnu, still OK
> for trunk? (Only change is the two adjusted testcases.)

OK.

> -- >8 --
> 
> Inline variables are vague-linkage, and may or may not need to be
> emitted in any TU that they are part of, similarly to e.g. template
> instantiations.
> 
> Currently 'import_export_decl' assumes that inline variables have
> already been emitted when it comes to end-of-TU processing, and so
> crashes when importing non-trivially-initialised variables from a
> module, as they have not yet been finalised.
> 
> This patch fixes this by ensuring that inline variables are always
> deferred till end-of-TU processing, unifying the behaviour for module
> and non-module code.
> 
> 	PR c++/113708
> 
> gcc/cp/ChangeLog:
> 
> 	* decl.cc (make_rtl_for_nonlocal_decl): Defer inline variables.
> 	* decl2.cc (import_export_decl): Support inline variables.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/debug/dwarf2/inline-var-1.C: Reference 'a' to ensure it
> 	is emitted.
> 	* g++.dg/debug/dwarf2/inline-var-3.C: Likewise.
> 	* g++.dg/modules/init-7_a.H: New test.
> 	* g++.dg/modules/init-7_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/decl.cc                                   | 4 ++++
>   gcc/cp/decl2.cc                                  | 7 +++++--
>   gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C | 2 ++
>   gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C | 2 ++
>   gcc/testsuite/g++.dg/modules/init-7_a.H          | 6 ++++++
>   gcc/testsuite/g++.dg/modules/init-7_b.C          | 6 ++++++
>   6 files changed, 25 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/init-7_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/init-7_b.C
> 
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 3e41fd4fa31..969513c069a 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -7954,6 +7954,10 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec)
>         && DECL_IMPLICIT_INSTANTIATION (decl))
>       defer_p = 1;
>   
> +  /* Defer vague-linkage variables.  */
> +  if (DECL_INLINE_VAR_P (decl))
> +    defer_p = 1;
> +
>     /* If we're not deferring, go ahead and assemble the variable.  */
>     if (!defer_p)
>       rest_of_decl_compilation (decl, toplev, at_eof);
> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
> index f569d4045ec..1dddbaab38b 100644
> --- a/gcc/cp/decl2.cc
> +++ b/gcc/cp/decl2.cc
> @@ -3360,7 +3360,9 @@ import_export_decl (tree decl)
>   
>        * implicit instantiations of function templates
>   
> -     * inline function
> +     * inline functions
> +
> +     * inline variables
>   
>        * implicit instantiations of static data members of class
>          templates
> @@ -3383,6 +3385,7 @@ import_export_decl (tree decl)
>   		|| DECL_DECLARED_INLINE_P (decl));
>     else
>       gcc_assert (DECL_IMPLICIT_INSTANTIATION (decl)
> +		|| DECL_INLINE_VAR_P (decl)
>   		|| DECL_VTABLE_OR_VTT_P (decl)
>   		|| DECL_TINFO_P (decl));
>     /* Check that a definition of DECL is available in this translation
> @@ -3511,7 +3514,7 @@ import_export_decl (tree decl)
>   	   this entity as undefined in this translation unit.  */
>   	import_p = true;
>       }
> -  else if (DECL_FUNCTION_MEMBER_P (decl))
> +  else if (TREE_CODE (decl) == FUNCTION_DECL && DECL_FUNCTION_MEMBER_P (decl))
>       {
>         if (!DECL_DECLARED_INLINE_P (decl))
>   	{
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
> index 85f74a91521..7ec20afc065 100644
> --- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-1.C
> @@ -8,6 +8,8 @@
>   // { dg-final { scan-assembler-times " DW_AT_\[^\n\r]*linkage_name" 7 } }
>   
>   inline int a;
> +int& ar = a;
> +
>   struct S
>   {
>     static inline double b = 4.0;
> diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
> index 6345b5e2ec8..bb40a229551 100644
> --- a/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
> +++ b/gcc/testsuite/g++.dg/debug/dwarf2/inline-var-3.C
> @@ -11,6 +11,8 @@
>   // { dg-final { scan-assembler-times " DW_AT_\[^\n\r]*linkage_name" 7 } }
>   
>   inline int a;
> +int& ar = a;
> +
>   struct S
>   {
>     static inline double b = 4.0;
> diff --git a/gcc/testsuite/g++.dg/modules/init-7_a.H b/gcc/testsuite/g++.dg/modules/init-7_a.H
> new file mode 100644
> index 00000000000..7a0bb721c30
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/init-7_a.H
> @@ -0,0 +1,6 @@
> +// PR c++/113708
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +inline int f() { return 42; }
> +inline int a = f();
> diff --git a/gcc/testsuite/g++.dg/modules/init-7_b.C b/gcc/testsuite/g++.dg/modules/init-7_b.C
> new file mode 100644
> index 00000000000..58bb0620ca5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/init-7_b.C
> @@ -0,0 +1,6 @@
> +// PR c++/113708
> +// { dg-module-do link }
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "init-7_a.H";
> +int main() { a; }


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-02-14 13:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-11 13:26 [PATCH] c++/modules: Finalise non-local imported vars [PR113708] Nathaniel Shead
2024-02-13 23:08 ` Jason Merrill
2024-02-14  1:34   ` [PATCH v2] c++: Defer emitting inline variables [PR113708] Nathaniel Shead
2024-02-14  2:47     ` Jason Merrill
2024-02-14 11:03       ` Nathaniel Shead
2024-02-14 13:34         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).