* [C++ PATCH] Fix -frepo (PR c++/34178) @ 2007-11-27 22:30 Jakub Jelinek 2007-12-05 15:14 ` [C++ PATCH] Fix -frepo (PR c++/34178, take 2) Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2007-11-27 22:30 UTC (permalink / raw) To: Mark Mitchell, Jason Merrill; +Cc: gcc-patches Hi! The testcase below doesn't link with -frepo, the static data member isn't defined anywhere. There are IMHO two separate issues - one is it doesn't make much sense to special case any constant expr initialized static data members, only const static data members with const expr initialization need to be present. The other is an interaction between repo_emit_p and import_export_decl. In some cases repo_emit_p decides to force emitting something, possibly in all compilation units, by returning 2. But that 2 is the same value repo_emit_p returns when -fno-repo. With -fno-repo -fno-implicit-templates we wouldn't want to instantiate e.g. the static data member in this case, but when repo_emit_p with -frepo (which implies -fno-implicit-templates) decided it should be emitted, not emitting it means it won't be in *.rpo either and therefore won't be added even during linking. Following was bootstrapped/regtested on x86_64-linux, ok for trunk? 2007-11-27 Jakub Jelinek <jakub@redhat.com> PR c++/34178 * repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P. * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when flag_use_repository and repo_emit_p returned 2. * g++.dg/template/repo6.C: New test. --- gcc/cp/repo.c.jj 2007-08-27 10:15:32.000000000 +0200 +++ gcc/cp/repo.c 2007-11-27 20:02:57.000000000 +0100 @@ -304,10 +304,10 @@ repo_emit_p (tree decl) && (!TYPE_LANG_SPECIFIC (type) || !CLASSTYPE_TEMPLATE_INSTANTIATION (type))) return 2; - /* Static data members initialized by constant expressions must + /* Const static data members initialized by constant expressions must be processed where needed so that their definitions are available. */ - if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) + if (DECL_INTEGRAL_CONSTANT_VAR_P (decl) && DECL_CLASS_SCOPE_P (decl)) return 2; } --- gcc/cp/decl2.c.jj 2007-11-26 22:19:54.000000000 +0100 +++ gcc/cp/decl2.c 2007-11-27 20:21:57.000000000 +0100 @@ -2230,7 +2230,8 @@ import_export_decl (tree decl) { /* DECL is an implicit instantiation of a function or static data member. */ - if (flag_implicit_templates + 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/repo6.C.jj 2007-11-27 20:59:36.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo6.C 2007-11-27 20:59:29.000000000 +0100 @@ -0,0 +1,25 @@ +// PR c++/34178 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +template<typename T> +class A +{ +private: + static const int x; + static int y; + +public: + int getX () { return x + y; } +}; + +template<typename T> const int A<T>::x = 0; +template<typename T> int A<T>::y = 0; + +int +main () +{ + A<int> a; + return a.getX(); +} Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* [C++ PATCH] Fix -frepo (PR c++/34178, take 2) 2007-11-27 22:30 [C++ PATCH] Fix -frepo (PR c++/34178) Jakub Jelinek @ 2007-12-05 15:14 ` Jakub Jelinek 2007-12-06 1:03 ` Mark Mitchell 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2007-12-05 15:14 UTC (permalink / raw) To: Mark Mitchell, Jason Merrill; +Cc: gcc-patches On Tue, Nov 27, 2007 at 04:58:31PM -0500, Jakub Jelinek wrote: > The testcase below doesn't link with -frepo, the static data member > isn't defined anywhere. > There are IMHO two separate issues - one is it doesn't make much sense > to special case any constant expr initialized static data members, only > const static data members with const expr initialization need to be > present. The other is an interaction between repo_emit_p and > import_export_decl. In some cases repo_emit_p decides to force emitting > something, possibly in all compilation units, by returning 2. But > that 2 is the same value repo_emit_p returns when -fno-repo. With > -fno-repo -fno-implicit-templates we wouldn't want to instantiate e.g. > the static data member in this case, but when repo_emit_p with -frepo > (which implies -fno-implicit-templates) decided it should be emitted, > not emitting it means it won't be in *.rpo either and therefore won't > be added even during linking. Turns out I was wrong, even const static data members initialized outside of the class should be always emitted, as can be seen e.g. on the new repo7.C testcase I have just posted. But for non-const static data members, even when initialized by constant expression we should handle them normally (i.e. with -frepo just note them in *.rpo and allow them to be compiled in when linker invokes us again). Regtested on x86_64-linux, ok for trunk? 2007-12-05 Jakub Jelinek <jakub@redhat.com> PR c++/34178 * repo.c (repo_emit_p): Return 2 for DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are const. * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when flag_use_repository and repo_emit_p returned 2. * g++.dg/template/repo6.C: New test. --- gcc/cp/decl2.c.jj 2007-12-05 13:11:12.000000000 +0100 +++ gcc/cp/decl2.c 2007-12-05 13:37:34.000000000 +0100 @@ -2230,7 +2230,8 @@ import_export_decl (tree decl) { /* DECL is an implicit instantiation of a function or static data member. */ - if (flag_implicit_templates + if ((flag_implicit_templates + && !flag_use_repository) || (flag_implicit_inline_templates && TREE_CODE (decl) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (decl))) --- gcc/cp/repo.c.jj 2007-12-04 20:33:17.000000000 +0100 +++ gcc/cp/repo.c 2007-12-05 14:40:36.000000000 +0100 @@ -304,11 +304,12 @@ repo_emit_p (tree decl) && (!TYPE_LANG_SPECIFIC (type) || !CLASSTYPE_TEMPLATE_INSTANTIATION (type))) return 2; - /* Static data members initialized by constant expressions must + /* Const static data members initialized by constant expressions must be processed where needed so that their definitions are available. */ if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) - && DECL_CLASS_SCOPE_P (decl)) + && DECL_CLASS_SCOPE_P (decl) + && CP_TYPE_CONST_P (TREE_TYPE (decl))) return 2; } else if (!DECL_TEMPLATE_INSTANTIATION (decl)) --- gcc/testsuite/g++.dg/template/repo6.C.jj 2007-12-04 20:44:19.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo6.C 2007-12-04 20:44:19.000000000 +0100 @@ -0,0 +1,25 @@ +// PR c++/34178 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +template<typename T> +class A +{ +private: + static const int x; + static int y; + +public: + int getX () { return x + y; } +}; + +template<typename T> const int A<T>::x = 0; +template<typename T> int A<T>::y = 0; + +int +main () +{ + A<int> a; + return a.getX(); +} Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2) 2007-12-05 15:14 ` [C++ PATCH] Fix -frepo (PR c++/34178, take 2) Jakub Jelinek @ 2007-12-06 1:03 ` Mark Mitchell 2007-12-06 10:48 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Mark Mitchell @ 2007-12-06 1:03 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches Jakub Jelinek wrote: > 2007-12-05 Jakub Jelinek <jakub@redhat.com> > > PR c++/34178 > * repo.c (repo_emit_p): Return 2 for > DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are > const. Why don't we want to use DECL_INTEGRAL_CONSTANT_VAR_P here? > * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when > flag_use_repository and repo_emit_p returned 2. > + if ((flag_implicit_templates > + && !flag_use_repository) Shouldn't that condition involve emit_p, rather than just be !flag_use_repository? Thanks, -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2) 2007-12-06 1:03 ` Mark Mitchell @ 2007-12-06 10:48 ` Jakub Jelinek 2007-12-07 4:28 ` Mark Mitchell 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2007-12-06 10:48 UTC (permalink / raw) To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches On Wed, Dec 05, 2007 at 05:03:29PM -0800, Mark Mitchell wrote: > Jakub Jelinek wrote: > > > 2007-12-05 Jakub Jelinek <jakub@redhat.com> > > > > PR c++/34178 > > * repo.c (repo_emit_p): Return 2 for > > DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are > > const. > > Why don't we want to use DECL_INTEGRAL_CONSTANT_VAR_P here? That's what my first patch version did. But that breaks e.g. the repo7.C testcase in http://gcc.gnu.org/ml/gcc-patches/2007-12/msg00182.html From the #define DECL_INTEGRAL_CONSTANT_VAR_P(NODE) \ (TREE_CODE (NODE) == VAR_DECL \ && CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (NODE)) \ && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (NODE)) \ && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (NODE)) conditions we already check TREE_CODE (decl) == VAR_DECL and DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) in repo.c and with this patch also CP_TYPE_CONST_P (TREE_TYPE (decl)) But in repo7.C the const static data member is aggregate, not integral or enum. finish_static_data_member has /* Force the compiler to know when an uninitialized static const member is being used. */ if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0) TREE_USED (decl) = 1; Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a definition was parsed) and after tsubst finish_static_data_member sets TREE_USED again. Next in instantiate_decl we call 14742 if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d)) on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that returns 0, which means the rest of instantiate_decl is bypassed for this decl. Then this makes into cgraph, which emits it, as the decl is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the initialized as common symbol, as DECL_INITIAL is NULL). > > * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when > > flag_use_repository and repo_emit_p returned 2. > > > + if ((flag_implicit_templates > > + && !flag_use_repository) > > Shouldn't that condition involve emit_p, rather than just be > !flag_use_repository? That's not needed, we know that emit_p is 2 at this point: emit_p = repo_emit_p (decl); if (emit_p == 0) import_p = true; else if (emit_p == 1) { ... return; } if (import_p) ; else if (...) { ... } else if (DECL_TEMPLATE_INSTANTIATION (decl) || DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (decl)) { // The && !flag_use_repository addition is here. } emit_p can be only 0, 1, 2, if it is 0, then import_p was true and so we don't hit this point, if it is 1, we returned early. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2) 2007-12-06 10:48 ` Jakub Jelinek @ 2007-12-07 4:28 ` Mark Mitchell 2007-12-07 10:35 ` [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Mark Mitchell @ 2007-12-07 4:28 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches Jakub Jelinek wrote: > But in repo7.C the const static data member is aggregate, not integral or > enum. finish_static_data_member has > /* Force the compiler to know when an uninitialized static const > member is being used. */ > if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0) > TREE_USED (decl) = 1; > Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a > definition was parsed) and after tsubst finish_static_data_member > sets TREE_USED again. Next in instantiate_decl we call > 14742 if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d)) > on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that > returns 0, which means the rest of instantiate_decl is bypassed for this > decl. Then this makes into cgraph, which emits it, as the decl > is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the > initialized as common symbol, as DECL_INITIAL is NULL). I'm confused. Are we talking about the second time we process the file, at link-time? And, at that point, does the .rpo file tells the compiler that it needs to define "A D<B>::b"? If so, repo_emit_p should be returning true anyhow. Otherwise, I think something is just going wrong with explicit template instantiation. Once we decide that D<B>::b is going to be emitted in the front end (i.e., clear DECL_EXTERNAL), we have to process its initializer. But, the fact that it's const doesn't seem relevant to me at all. -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) 2007-12-07 4:28 ` Mark Mitchell @ 2007-12-07 10:35 ` Jakub Jelinek 2007-12-07 17:55 ` Mark Mitchell 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2007-12-07 10:35 UTC (permalink / raw) To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches On Thu, Dec 06, 2007 at 08:27:55PM -0800, Mark Mitchell wrote: > > But in repo7.C the const static data member is aggregate, not integral or > > enum. finish_static_data_member has > > /* Force the compiler to know when an uninitialized static const > > member is being used. */ > > if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0) > > TREE_USED (decl) = 1; > > Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a > > definition was parsed) and after tsubst finish_static_data_member > > sets TREE_USED again. Next in instantiate_decl we call > > 14742 if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d)) > > on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that > > returns 0, which means the rest of instantiate_decl is bypassed for this > > decl. Then this makes into cgraph, which emits it, as the decl > > is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the > > initialized as common symbol, as DECL_INITIAL is NULL). > > I'm confused. Are we talking about the second time we process the file, > at link-time? No, I'm talking about the original compilation, which creates the .rpo file. Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is: struct A { int a; }; template <typename T> struct D { static const A b; }; template<typename T> const A D<T>::b = { 2 }; template class D<A>; int main () { } Here there is just one static data member. finish_static_data_member_decl marks D<T>::b as TREE_USED because it is const. duplicate_decls is called on: olddecl <var_decl 0x2aaaaea7f000 b type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI size <integer_cst 0x2aaaae927a50 constant invariant 32> unit size <integer_cst 0x2aaaae9276c0 constant invariant 4> align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40 fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int> nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 offset_align 128 offset <integer_cst 0x2aaaae9276f0 constant invariant 0> bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown> used public static external decl_3 decl_6 SI file repo8.C line 13 col 18 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 context <record_type 0x2aaaaea75780 D> template-info 0x2aaaaea6bde0 chain <type_decl 0x2aaaaea759c0 D>> newdecl <var_decl 0x2aaaaea7f0a0 b type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI size <integer_cst 0x2aaaae927a50 constant invariant 32> unit size <integer_cst 0x2aaaae9276c0 constant invariant 4> align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40 fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int> nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 offset_align 128 offset <integer_cst 0x2aaaae9276f0 constant invariant 0> bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown> public static SI file repo8.C line 16 col 36 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 context <record_type 0x2aaaaea75780 D> > so it still has DECL_AGGR_P set, but is no longer external. start_decl later clears DECL_IN_AGGR_P on this. Then tsubst_decl creates a new var_decl: <var_decl 0x2aaaaea7f1e0 b type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI size <integer_cst 0x2aaaae927a50 constant invariant 32> unit size <integer_cst 0x2aaaae9276c0 constant invariant 4> align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40 fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int> nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 offset_align 128 offset <integer_cst 0x2aaaae9276f0 constant invariant 0> bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>> X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown> readonly used public static tree_2 external SI file repo8.C line 16 col 36 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4> align 32 context <record_type 0x2aaaaea75e40 D> template-info 0x2aaaaea803c0> and finish_static_data_member_decl on it 737 if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0) 738 TREE_USED (decl) = 1; 739 DECL_INITIAL (decl) = init; 740 DECL_IN_AGGR_P (decl) = 1; instantiate_class_member -> mark_decl_instantiated marks this as instantiated and finally instantiate_class_member -> instantiate_decl calls that 14742 if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d)) If repo_emit_p doesn't return 0 in this case, then instantiate_decl would later do: /* Clear out DECL_RTL; whatever was there before may not be right since we've reset the type of the declaration. */ SET_DECL_RTL (d, NULL_RTX); DECL_IN_AGGR_P (d) = 0; /* The initializer is placed in DECL_INITIAL by regenerate_decl_from_template. Pull it out so that finish_decl can process it. */ init = DECL_INITIAL (d); DECL_INITIAL (d) = NULL_TREE; DECL_INITIALIZED_P (d) = 0; /* Clear DECL_EXTERNAL so that cp_finish_decl will process the initializer. That function will defer actual emission until we have a chance to determine linkage. */ DECL_EXTERNAL (d) = 0; /* Enter the scope of D so that access-checking works correctly. */ push_nested_class (DECL_CONTEXT (d)); finish_decl (d, init, NULL_TREE); pop_nested_class (); But as repo_emit_p above returned 0, it will just go out, but 0x2aaaaea7f1e0 is TREE_USED, !DECL_EXTERNAL so it will be emitted, even without initializer (because regenerate_decl_from_template which sets DECL_INITIAL has not been called either). If I explicitly make d DECL_EXTERNAL again when repo_emit_p returns 0, this testcase and even one with additional const A *x = &D<B>::b; compiles, doesn't contain _ZN1DI1BE1bE definition (only references it in x var): rm -f repo7{.o,.s,.rpo}; ./g++ -B ./ -frepo -c repo7.C; nm repo7.o; ./g++ -B ./ -frepo repo7.o -o repo7 -L ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/; cat repo7.rpo; nm repo7 | grep ' _Z' 0000000000000000 R _ZN1B1bE U _ZN1DI1BE1bE U __gxx_personality_v0 0000000000000000 T main 0000000000000000 D x collect: recompiling repo7.C collect: relinking M repo7.C D /usr/src/gcc/obj/gcc A '-B' './' '-frepo' '-c' '-shared-libgcc' '-mtune=generic' '-frandom-seed=0xb3d94816' '-shared-libgcc' C _ZN1DI1BE1bE 000000000040063c R _ZN1B1bE 0000000000400640 V _ZN1DI1BE1bE which looks ok. Making it DECL_EXTERNAL and !DECL_NOT_REALLY_EXTERN matches what import_export_decl (the only other caller of repo_emit_p) is already doing if repo_emit_p returns 0 - sets import_p and later: if (import_p) { /* If we are importing DECL into this translation unit, mark is an undefined here. */ DECL_EXTERNAL (decl) = 1; DECL_NOT_REALLY_EXTERN (decl) = 0; } I have regtested following patch on x86_64-linux, ok for trunk? 2007-12-07 Jakub Jelinek <jakub@redhat.com> PR c++/34178 PR c++/34340 * repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P. * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when flag_use_repository and repo_emit_p returned 2. * pt.c (instantiate_decl): If repo_emit_p returned 0, mark d as DECL_EXTERNAL and !DECL_NOT_REALLY_EXTERN. * g++.dg/template/repo6.C: New test. * g++.dg/template/repo7.C: New test. --- gcc/cp/pt.c.jj 2007-12-05 21:42:08.000000000 +0100 +++ gcc/cp/pt.c 2007-12-07 11:11:22.000000000 +0100 @@ -14753,7 +14753,14 @@ instantiate_decl (tree d, int defer_ok, if (!(TREE_CODE (d) == FUNCTION_DECL && flag_inline_trees && DECL_DECLARED_INLINE_P (d))) - goto out; + { + /* If we are importing DECL into this translation unit, mark it + as undefined here. */ + DECL_EXTERNAL (d) = 1; + DECL_NOT_REALLY_EXTERN (d) = 0; + DECL_INTERFACE_KNOWN (d) = 1; + goto out; + } } need_push = !cfun || !global_bindings_p (); --- gcc/cp/decl2.c.jj 2007-12-06 12:00:12.000000000 +0100 +++ gcc/cp/decl2.c 2007-12-06 12:00:19.000000000 +0100 @@ -2230,7 +2230,8 @@ import_export_decl (tree decl) { /* DECL is an implicit instantiation of a function or static data member. */ - if (flag_implicit_templates + if ((flag_implicit_templates + && !flag_use_repository) || (flag_implicit_inline_templates && TREE_CODE (decl) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (decl))) --- gcc/cp/repo.c.jj 2007-12-06 11:53:40.000000000 +0100 +++ gcc/cp/repo.c 2007-12-07 09:35:46.000000000 +0100 @@ -304,10 +304,10 @@ repo_emit_p (tree decl) && (!TYPE_LANG_SPECIFIC (type) || !CLASSTYPE_TEMPLATE_INSTANTIATION (type))) return 2; - /* Static data members initialized by constant expressions must + /* Const static data members initialized by constant expressions must be processed where needed so that their definitions are available. */ - if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) + if (DECL_INTEGRAL_CONSTANT_VAR_P (decl) && DECL_CLASS_SCOPE_P (decl)) return 2; } --- gcc/testsuite/g++.dg/template/repo6.C.jj 2007-12-06 12:00:19.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo6.C 2007-12-06 12:00:19.000000000 +0100 @@ -0,0 +1,25 @@ +// PR c++/34178 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +template<typename T> +class A +{ +private: + static const int x; + static int y; + +public: + int getX () { return x + y; } +}; + +template<typename T> const int A<T>::x = 0; +template<typename T> int A<T>::y = 0; + +int +main () +{ + A<int> a; + return a.getX(); +} --- gcc/testsuite/g++.dg/template/repo7.C.jj 2007-12-06 12:00:12.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo7.C 2007-12-07 11:07:53.000000000 +0100 @@ -0,0 +1,24 @@ +// PR c++/34340 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +struct A +{ + int a; +}; + +template <typename T> struct D +{ + static const A b; +}; + +template<typename T> const A D<T>::b = { 2 }; +template class D<A>; + +const A *x = &D<A>::b; + +int +main () +{ +} Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) 2007-12-07 10:35 ` [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) Jakub Jelinek @ 2007-12-07 17:55 ` Mark Mitchell 2007-12-07 22:53 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Mark Mitchell @ 2007-12-07 17:55 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches Jakub Jelinek wrote: > No, I'm talking about the original compilation, which creates the .rpo file. > > Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is: > struct A { int a; }; > template <typename T> struct D { static const A b; }; > template<typename T> const A D<T>::b = { 2 }; > template class D<A>; > int main () { } > > Here there is just one static data member. > finish_static_data_member_decl marks D<T>::b as TREE_USED because it is > const. Do you really mean D<T> or D<A>? TREE_USED on D<T>::b should not matter as that's not a real thing; only D<A>::b might show up in the object file. Anyhow, the explicit instantiation directive should result in us emitting D<A>::b. I don't think that -frepo should cause us not to emit things that the user has explicitly instantiated. I would expect that the right code in repo_emit_p would be: /* Variables that can appear in integral constant-expressions must be instantiated so that the values of th variables are available. */ if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)) return 2; /* Explicit instantiations should be handled normally even when using a repository. */ if (DECL_EXPLICIT_INSTANTIATION (decl)) return 2; Does that work? -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) 2007-12-07 17:55 ` Mark Mitchell @ 2007-12-07 22:53 ` Jakub Jelinek 2007-12-08 1:27 ` Mark Mitchell 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2007-12-07 22:53 UTC (permalink / raw) To: Mark Mitchell; +Cc: Jason Merrill, gcc-patches On Fri, Dec 07, 2007 at 09:55:44AM -0800, Mark Mitchell wrote: > Jakub Jelinek wrote: > > > No, I'm talking about the original compilation, which creates the .rpo file. > > > > Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is: > > struct A { int a; }; > > template <typename T> struct D { static const A b; }; > > template<typename T> const A D<T>::b = { 2 }; > > template class D<A>; > > int main () { } > > > > Here there is just one static data member. > > finish_static_data_member_decl marks D<T>::b as TREE_USED because it is > > const. > > Do you really mean D<T> or D<A>? TREE_USED on D<T>::b should not matter > as that's not a real thing; only D<A>::b might show up in the object file. I meant of course D<A>, sorry. > Anyhow, the explicit instantiation directive should result in us > emitting D<A>::b. I don't think that -frepo should cause us not to emit > things that the user has explicitly instantiated. > > I would expect that the right code in repo_emit_p would be: > > /* Variables that can appear in integral constant-expressions must > be instantiated so that the values of th variables are > available. */ > if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)) > return 2; > /* Explicit instantiations should be handled normally even when using > a repository. */ > if (DECL_EXPLICIT_INSTANTIATION (decl)) > return 2; > > Does that work? That seems to work too, for make check-g++ at least (but there are just very few -frepo testcases, which is probably why this could be broken for so long). 2007-12-07 Jakub Jelinek <jakub@redhat.com> PR c++/34178 PR c++/34340 * repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P. Return 2 also if DECL_EXPLICIT_INSTANTIATION. * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when flag_use_repository and repo_emit_p returned 2. * g++.dg/template/repo6.C: New test. * g++.dg/template/repo7.C: New test. * g++.dg/template/repo8.C: New test. --- gcc/cp/decl2.c.jj 2007-12-07 12:21:03.000000000 +0100 +++ gcc/cp/decl2.c 2007-12-07 22:24:35.000000000 +0100 @@ -2230,7 +2230,8 @@ import_export_decl (tree decl) { /* DECL is an implicit instantiation of a function or static data member. */ - if (flag_implicit_templates + if ((flag_implicit_templates + && !flag_use_repository) || (flag_implicit_inline_templates && TREE_CODE (decl) == FUNCTION_DECL && DECL_DECLARED_INLINE_P (decl))) --- gcc/cp/repo.c.jj 2007-12-07 12:21:03.000000000 +0100 +++ gcc/cp/repo.c 2007-12-07 22:26:43.000000000 +0100 @@ -304,16 +304,19 @@ repo_emit_p (tree decl) && (!TYPE_LANG_SPECIFIC (type) || !CLASSTYPE_TEMPLATE_INSTANTIATION (type))) return 2; - /* Static data members initialized by constant expressions must + /* Const static data members initialized by constant expressions must be processed where needed so that their definitions are available. */ - if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) + if (DECL_INTEGRAL_CONSTANT_VAR_P (decl) && DECL_CLASS_SCOPE_P (decl)) return 2; } else if (!DECL_TEMPLATE_INSTANTIATION (decl)) return 2; + if (DECL_EXPLICIT_INSTANTIATION (decl)) + return 2; + /* For constructors and destructors, the repository contains information about the clones -- not the original function -- because only the clones are emitted in the object file. */ --- gcc/testsuite/g++.dg/template/repo6.C.jj 2007-12-07 22:24:35.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo6.C 2007-12-07 22:24:35.000000000 +0100 @@ -0,0 +1,25 @@ +// PR c++/34178 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +template<typename T> +class A +{ +private: + static const int x; + static int y; + +public: + int getX () { return x + y; } +}; + +template<typename T> const int A<T>::x = 0; +template<typename T> int A<T>::y = 0; + +int +main () +{ + A<int> a; + return a.getX(); +} --- gcc/testsuite/g++.dg/template/repo7.C.jj 2007-12-07 22:24:35.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo7.C 2007-12-07 22:24:35.000000000 +0100 @@ -0,0 +1,24 @@ +// PR c++/34340 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +struct A +{ + int a; +}; + +template <typename T> struct D +{ + static const A b; +}; + +template<typename T> const A D<T>::b = { 2 }; +template class D<A>; + +const A *x = &D<A>::b; + +int +main () +{ +} --- gcc/testsuite/g++.dg/template/repo8.C.jj 2007-12-07 22:24:35.000000000 +0100 +++ gcc/testsuite/g++.dg/template/repo8.C 2007-12-07 22:24:35.000000000 +0100 @@ -0,0 +1,23 @@ +// PR c++/34340 +// { dg-options "-frepo" } +// { dg-final { cleanup-repo-files } } +// { dg-require-host-local "" } + +struct A +{ + int a; +}; + +template <typename T> struct D +{ + static const A b; +}; + +template<typename T> const A D<T>::b = { 2 }; + +const A *x = &D<A>::b; + +int +main () +{ +} Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) 2007-12-07 22:53 ` Jakub Jelinek @ 2007-12-08 1:27 ` Mark Mitchell 0 siblings, 0 replies; 9+ messages in thread From: Mark Mitchell @ 2007-12-08 1:27 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches Jakub Jelinek wrote: > That seems to work too, for make check-g++ at least (but there are just > very few -frepo testcases, which is probably why this could be broken for so > long). > PR c++/34178 > PR c++/34340 > * repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P > in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P. > Return 2 also if DECL_EXPLICIT_INSTANTIATION. > * decl2.c (import_export_decl): Don't make VAR_DECLs import_p when > flag_use_repository and repo_emit_p returned 2. > > * g++.dg/template/repo6.C: New test. > * g++.dg/template/repo7.C: New test. > * g++.dg/template/repo8.C: New test. If Jason doesn't object within 24 hours, please go ahead and put this in. This seems sensible (at least to me!) so I think it's worth a try. Thanks, -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-12-08 1:27 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-11-27 22:30 [C++ PATCH] Fix -frepo (PR c++/34178) Jakub Jelinek 2007-12-05 15:14 ` [C++ PATCH] Fix -frepo (PR c++/34178, take 2) Jakub Jelinek 2007-12-06 1:03 ` Mark Mitchell 2007-12-06 10:48 ` Jakub Jelinek 2007-12-07 4:28 ` Mark Mitchell 2007-12-07 10:35 ` [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3) Jakub Jelinek 2007-12-07 17:55 ` Mark Mitchell 2007-12-07 22:53 ` Jakub Jelinek 2007-12-08 1:27 ` 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).