Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
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
---
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
Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
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
---
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 "
Re: [PATCH v2] c++: Defer emitting inline variables [PR113708]
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
---
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 000..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 000..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; }
