* [C++ PATCH] Fix -frepo (PR c++/36364)
@ 2008-06-23 16:07 Jakub Jelinek
2008-06-24 21:23 ` Mark Mitchell
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2008-06-23 16:07 UTC (permalink / raw)
To: Jason Merrill, Mark Mitchell; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]
Hi!
The following testcase distilled from:
#include <string>
#include <ext/mt_allocator.h>
//workaround:
//template class std::basic_string <char, std::char_traits <char>, __gnu_cxx::__mt_alloc <char> >;
int main ()
{
std::basic_string <char, std::char_traits <char>, __gnu_cxx::__mt_alloc <char> > s;
s.push_back ('a');
}
doesn't link with -frepo. The problem is a static const data member with
initializer, which repo_emit_p returns 2 for (to make sure it is properly
optimized out when possible), but then import_export_decl sees the
-frepo implied -fno-implicit-templates and doesn't emit it at all.
Saying this is invalid and requires user to explicitly instantiate it is
difficult, as that IMHO doesn't play well with the -frepo model.
Attached are two alternative patches. One stops implying
-fno-implicit-templates for -frepo in c-opts.c, only implies that
in import_export_class and in import_export_decl for VTABLE/VTT, the other in
the second spot in import_export_decl basically implies -fimplicit-templates
for the cases where repo_emit_p returned 2.
The first one will require explicit instantiation for terminal
if -frepo -fno-implicit-templates, the second one will not, otherwise
they are the same.
The second line in
if ((flag_implicit_templates
&& !flag_use_repository)
I've added in December last year doesn't make sense, in current
SVN flag_use_repository != 0 implies flag_implicit_templates = 0.
Is this ok for 4.4/4.3, or do you prefer doing something else?
Jakub
[-- Attachment #2: Z197 --]
[-- Type: text/plain, Size: 3246 bytes --]
2008-06-23 Jakub Jelinek <jakub@redhat.com>
PR c++/36364
* c-opts.c (c_common_handle_option): Don't clear
flag_implicit_templates for -frepo.
* decl2.c (import_export_class): Adjust for -frepo
not implying flag_implicit_templates = 0.
(import_export_decl): Likewise.
* g++.dg/template/repo9.C: New test.
--- gcc/c-opts.c.jj 2008-06-19 12:22:29.000000000 +0200
+++ gcc/c-opts.c 2008-06-23 16:44:10.000000000 +0200
@@ -755,8 +755,6 @@ c_common_handle_option (size_t scode, co
case OPT_frepo:
flag_use_repository = value;
- if (value)
- flag_implicit_templates = 0;
break;
case OPT_frtti:
--- gcc/cp/decl2.c.jj 2008-06-23 08:40:29.000000000 +0200
+++ gcc/cp/decl2.c 2008-06-23 16:45:41.000000000 +0200
@@ -1589,7 +1589,7 @@ import_export_class (tree ctype)
else if (lookup_attribute ("dllexport", TYPE_ATTRIBUTES (ctype)))
import_export = 1;
else if (CLASSTYPE_IMPLICIT_INSTANTIATION (ctype)
- && !flag_implicit_templates)
+ && (!flag_implicit_templates || flag_use_repository))
/* For a template class, without -fimplicit-templates, check the
repository. If the virtual table is assigned to this
translation unit, then export the class; otherwise, import
@@ -2282,7 +2282,7 @@ import_export_decl (tree decl)
}
}
}
- else if (!flag_implicit_templates
+ else if ((!flag_implicit_templates || flag_use_repository)
&& CLASSTYPE_IMPLICIT_INSTANTIATION (class_type))
import_p = true;
else
@@ -2331,8 +2331,7 @@ import_export_decl (tree decl)
{
/* DECL is an implicit instantiation of a function or static
data member. */
- if ((flag_implicit_templates
- && !flag_use_repository)
+ if (flag_implicit_templates
|| (flag_implicit_inline_templates
&& TREE_CODE (decl) == FUNCTION_DECL
&& DECL_DECLARED_INLINE_P (decl)))
--- gcc/testsuite/g++.dg/template/repo9.C.jj 2008-06-23 16:54:25.000000000 +0200
+++ gcc/testsuite/g++.dg/template/repo9.C 2008-06-23 16:54:14.000000000 +0200
@@ -0,0 +1,48 @@
+// PR c++/36364
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template <typename C> struct A
+{
+ static void assign (C &c1, const C &c2) { c1 = c2; }
+};
+
+template <typename C, typename T> struct B
+{
+ struct D
+ {
+ static const C terminal;
+ static unsigned long stor[];
+ static D &empty_rep ()
+ {
+ void *p = reinterpret_cast <void *>(&stor);
+ return *reinterpret_cast <D *>(p);
+ }
+ void test (unsigned long n)
+ {
+ T::assign (this->refdata ()[n], terminal);
+ }
+ C *refdata () throw ()
+ {
+ return reinterpret_cast <C *>(this + 1);
+ }
+ };
+ C *dataplus;
+ C *data () const { return dataplus; }
+ D *rep () const { return &((reinterpret_cast < D * >(data ()))[-1]); }
+ static D & empty_rep () { return D::empty_rep (); }
+ B () : dataplus (empty_rep ().refdata ()) { }
+ ~B () { }
+ void push_back (C c) { rep ()->test (10); }
+};
+
+template <typename C, typename T> const C B <C, T>::D::terminal = C ();
+template <typename C, typename T> unsigned long B <C, T>::D::stor[64];
+
+int
+main ()
+{
+ B <char, A <char> > s;
+ s.push_back ('a');
+}
[-- Attachment #3: Z197a --]
[-- Type: text/plain, Size: 2045 bytes --]
2008-06-23 Jakub Jelinek <jakub@redhat.com>
PR c++/36364
* decl2.c (import_export_decl): Set comdat_p whenever -frepo.
* g++.dg/template/repo9.C: New test.
--- gcc/cp/decl2.c.jj 2008-06-23 08:40:29.000000000 +0200
+++ gcc/cp/decl2.c 2008-06-23 17:33:53.000000000 +0200
@@ -2331,8 +2331,8 @@ import_export_decl (tree decl)
{
/* DECL is an implicit instantiation of a function or static
data member. */
- if ((flag_implicit_templates
- && !flag_use_repository)
+ if (flag_implicit_templates
+ || flag_use_repository
|| (flag_implicit_inline_templates
&& TREE_CODE (decl) == FUNCTION_DECL
&& DECL_DECLARED_INLINE_P (decl)))
--- gcc/testsuite/g++.dg/template/repo9.C.jj 2008-06-23 16:54:25.000000000 +0200
+++ gcc/testsuite/g++.dg/template/repo9.C 2008-06-23 16:54:14.000000000 +0200
@@ -0,0 +1,48 @@
+// PR c++/36364
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template <typename C> struct A
+{
+ static void assign (C &c1, const C &c2) { c1 = c2; }
+};
+
+template <typename C, typename T> struct B
+{
+ struct D
+ {
+ static const C terminal;
+ static unsigned long stor[];
+ static D &empty_rep ()
+ {
+ void *p = reinterpret_cast <void *>(&stor);
+ return *reinterpret_cast <D *>(p);
+ }
+ void test (unsigned long n)
+ {
+ T::assign (this->refdata ()[n], terminal);
+ }
+ C *refdata () throw ()
+ {
+ return reinterpret_cast <C *>(this + 1);
+ }
+ };
+ C *dataplus;
+ C *data () const { return dataplus; }
+ D *rep () const { return &((reinterpret_cast < D * >(data ()))[-1]); }
+ static D & empty_rep () { return D::empty_rep (); }
+ B () : dataplus (empty_rep ().refdata ()) { }
+ ~B () { }
+ void push_back (C c) { rep ()->test (10); }
+};
+
+template <typename C, typename T> const C B <C, T>::D::terminal = C ();
+template <typename C, typename T> unsigned long B <C, T>::D::stor[64];
+
+int
+main ()
+{
+ B <char, A <char> > s;
+ s.push_back ('a');
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/36364)
2008-06-23 16:07 [C++ PATCH] Fix -frepo (PR c++/36364) Jakub Jelinek
@ 2008-06-24 21:23 ` Mark Mitchell
2008-06-24 22:54 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Mark Mitchell @ 2008-06-24 21:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
Jakub Jelinek wrote:
> std::basic_string <char, std::char_traits <char>, __gnu_cxx::__mt_alloc <char> > s;
> doesn't link with -frepo. The problem is a static const data member with
> initializer, which repo_emit_p returns 2 for (to make sure it is properly
> optimized out when possible), but then import_export_decl sees the
> -frepo implied -fno-implicit-templates and doesn't emit it at all.
If I understand correctly, the problem is that we have a static data
member of a template which is marked as required by collect2, but then
when we recompile the file, is not actually emitted. Is that right?
If so, I think the fix is to honor IDENTIFIER_REPO_CHOSEN for the
VAR_DECL case of a static data member initialized by an integral
constant expression. Change the code in this test:
if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
&& DECL_CLASS_SCOPE_P (decl))
to (1) add the variable to pending_repo, and (2) if
IDENTIFIER_REPO_CHOSEN return 1, otherwise return 2. Probably the:
if (!DECL_REPO_AVAILABLE...)
block should be factored into a helper function.
Does that work?
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/36364)
2008-06-24 21:23 ` Mark Mitchell
@ 2008-06-24 22:54 ` Jakub Jelinek
2008-06-24 23:03 ` Mark Mitchell
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2008-06-24 22:54 UTC (permalink / raw)
To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches
On Tue, Jun 24, 2008 at 02:03:37PM -0700, Mark Mitchell wrote:
> Jakub Jelinek wrote:
>
> > std::basic_string <char, std::char_traits <char>, __gnu_cxx::__mt_alloc
> > <char> > s;
>
> >doesn't link with -frepo. The problem is a static const data member with
> >initializer, which repo_emit_p returns 2 for (to make sure it is properly
> >optimized out when possible), but then import_export_decl sees the
> >-frepo implied -fno-implicit-templates and doesn't emit it at all.
>
> If I understand correctly, the problem is that we have a static data
> member of a template which is marked as required by collect2, but then
> when we recompile the file, is not actually emitted. Is that right?
No. The problem is const static data member, for which:
/* Const static data members initialized by constant expressions must
be processed where needed so that their definitions are
available. */
if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
&& DECL_CLASS_SCOPE_P (decl))
return 2;
Returning 2 from repo_emit_p means thatg it is handled as in -fno-repo
compilation - it never makes it into the *.rpo file, neither
as O _Z...terminal, nor as C _Z...terminal. But as -frepo implies
-fno-implicit-templates, in this case it works as in -fno-implicit-templates
compilation - which means explicit instantiation is required.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/36364)
2008-06-24 22:54 ` Jakub Jelinek
@ 2008-06-24 23:03 ` Mark Mitchell
2008-06-27 15:57 ` [C++ PATCH] Fix -frepo (PR c++/36364, take 2) Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Mark Mitchell @ 2008-06-24 23:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
Jakub Jelinek wrote:
> No. The problem is const static data member, for which:
> /* Const static data members initialized by constant expressions must
> be processed where needed so that their definitions are
> available. */
> if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
> && DECL_CLASS_SCOPE_P (decl))
> return 2;
> Returning 2 from repo_emit_p means thatg it is handled as in -fno-repo
> compilation - it never makes it into the *.rpo file, neither
> as O _Z...terminal, nor as C _Z...terminal. But as -frepo implies
> -fno-implicit-templates, in this case it works as in -fno-implicit-templates
> compilation - which means explicit instantiation is required.
Right, that is why I suggested using both the DECL_REPO_AVAILABLE and
the IDENTIFIER_REPO_CHOSEN logic. Of course, you have to use
DECL_REPO_AVAILABLE to let collect2 know about it. But, once you do
that, it should assign it to an object file, and then
IDENTIFIER_REPO_CHOSEN will tell you to write it out by returning 1 from
that function.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 6+ messages in thread
* [C++ PATCH] Fix -frepo (PR c++/36364, take 2)
2008-06-24 23:03 ` Mark Mitchell
@ 2008-06-27 15:57 ` Jakub Jelinek
2008-06-27 22:33 ` Mark Mitchell
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2008-06-27 15:57 UTC (permalink / raw)
To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches
On Tue, Jun 24, 2008 at 03:53:46PM -0700, Mark Mitchell wrote:
> Jakub Jelinek wrote:
>
> >No. The problem is const static data member, for which:
> > /* Const static data members initialized by constant expressions must
> > be processed where needed so that their definitions are
> > available. */
> > if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
> > && DECL_CLASS_SCOPE_P (decl))
> > return 2;
> >Returning 2 from repo_emit_p means thatg it is handled as in -fno-repo
> >compilation - it never makes it into the *.rpo file, neither
> >as O _Z...terminal, nor as C _Z...terminal. But as -frepo implies
> >-fno-implicit-templates, in this case it works as in
> >-fno-implicit-templates
> >compilation - which means explicit instantiation is required.
>
> Right, that is why I suggested using both the DECL_REPO_AVAILABLE and
> the IDENTIFIER_REPO_CHOSEN logic. Of course, you have to use
> DECL_REPO_AVAILABLE to let collect2 know about it. But, once you do
> that, it should assign it to an object file, and then
> IDENTIFIER_REPO_CHOSEN will tell you to write it out by returning 1 from
> that function.
Did you mean something like this, or have I completely misunderstood?
Works on RUNTESTFLAGS=dg.exp=repo*.C, will run a full x86_64-linux
bootstrap/regtest tonight.
Ok for trunk/4.3?
2008-06-27 Jakub Jelinek <jakub@redhat.com>
PR c++/36364
* repo.c (repo_emit_p): Put const static data members initialized
by const expr into *.rpo file, just return 2 if IDENTIFIER_REPO_CHOSEN
for it is 0.
* g++.dg/template/repo9.C: New test.
--- gcc/cp/repo.c.jj 2008-02-11 14:45:57.000000000 +0100
+++ gcc/cp/repo.c 2008-06-27 17:16:23.000000000 +0200
@@ -280,6 +280,7 @@ finish_repo (void)
int
repo_emit_p (tree decl)
{
+ int ret = 0;
gcc_assert (TREE_PUBLIC (decl));
gcc_assert (TREE_CODE (decl) == FUNCTION_DECL
|| TREE_CODE (decl) == VAR_DECL);
@@ -306,10 +307,12 @@ repo_emit_p (tree decl)
return 2;
/* Const static data members initialized by constant expressions must
be processed where needed so that their definitions are
- available. */
+ available. Still record them into *.rpo files, so if they
+ weren't actually emitted and collect2 requests them, they can
+ be provided. */
if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
&& DECL_CLASS_SCOPE_P (decl))
- return 2;
+ ret = 2;
}
else if (!DECL_TEMPLATE_INSTANTIATION (decl))
return 2;
@@ -343,7 +346,7 @@ repo_emit_p (tree decl)
pending_repo = tree_cons (NULL_TREE, decl, pending_repo);
}
- return IDENTIFIER_REPO_CHOSEN (DECL_ASSEMBLER_NAME (decl));
+ return IDENTIFIER_REPO_CHOSEN (DECL_ASSEMBLER_NAME (decl)) ? 1 : ret;
}
/* Returns true iff the prelinker has explicitly marked CLASS_TYPE for
--- gcc/testsuite/g++.dg/template/repo9.C.jj 2008-06-27 16:56:42.000000000 +0200
+++ gcc/testsuite/g++.dg/template/repo9.C 2008-06-27 16:56:42.000000000 +0200
@@ -0,0 +1,48 @@
+// PR c++/36364
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template <typename C> struct A
+{
+ static void assign (C &c1, const C &c2) { c1 = c2; }
+};
+
+template <typename C, typename T> struct B
+{
+ struct D
+ {
+ static const C terminal;
+ static unsigned long stor[];
+ static D &empty_rep ()
+ {
+ void *p = reinterpret_cast <void *>(&stor);
+ return *reinterpret_cast <D *>(p);
+ }
+ void test (unsigned long n)
+ {
+ T::assign (this->refdata ()[n], terminal);
+ }
+ C *refdata () throw ()
+ {
+ return reinterpret_cast <C *>(this + 1);
+ }
+ };
+ C *dataplus;
+ C *data () const { return dataplus; }
+ D *rep () const { return &((reinterpret_cast < D * >(data ()))[-1]); }
+ static D & empty_rep () { return D::empty_rep (); }
+ B () : dataplus (empty_rep ().refdata ()) { }
+ ~B () { }
+ void push_back (C c) { rep ()->test (10); }
+};
+
+template <typename C, typename T> const C B <C, T>::D::terminal = C ();
+template <typename C, typename T> unsigned long B <C, T>::D::stor[64];
+
+int
+main ()
+{
+ B <char, A <char> > s;
+ s.push_back ('a');
+}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/36364, take 2)
2008-06-27 15:57 ` [C++ PATCH] Fix -frepo (PR c++/36364, take 2) Jakub Jelinek
@ 2008-06-27 22:33 ` Mark Mitchell
0 siblings, 0 replies; 6+ messages in thread
From: Mark Mitchell @ 2008-06-27 22:33 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
Jakub Jelinek wrote:
> Did you mean something like this, or have I completely misunderstood?
No, that's exactly what I meant.
> Works on RUNTESTFLAGS=dg.exp=repo*.C, will run a full x86_64-linux
> bootstrap/regtest tonight.
>
> Ok for trunk/4.3?
OK, assuming testing shows no new failures.
> 2008-06-27 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/36364
> * repo.c (repo_emit_p): Put const static data members initialized
> by const expr into *.rpo file, just return 2 if IDENTIFIER_REPO_CHOSEN
> for it is 0.
>
> * g++.dg/template/repo9.C: New test.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-06-27 22:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-23 16:07 [C++ PATCH] Fix -frepo (PR c++/36364) Jakub Jelinek
2008-06-24 21:23 ` Mark Mitchell
2008-06-24 22:54 ` Jakub Jelinek
2008-06-24 23:03 ` Mark Mitchell
2008-06-27 15:57 ` [C++ PATCH] Fix -frepo (PR c++/36364, take 2) Jakub Jelinek
2008-06-27 22:33 ` Mark Mitchell
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).