ychen created this revision. ychen added reviewers: rsmith, rnk. Herald added a project: All. ychen requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Based on the Richard's suggestion in D126341 <https://reviews.llvm.org/D126341>: `If we can actually describe a rule that we provide for initialization order of instantiated variables, and we can easily implement that rule and be confident we won't want to substantially weaken it later, and we can thereby assure our users that we will satisfy that rule, then I think that could be interesting, but anything less than that doesn't seem worthwhile to me.` I'm giving it try here. IMHO the implementation is pretty simple and does not change behavior for unrelated constructs like the timing when instantiated variable are passed to CodeGen. This is based on the same ordering guarantee needed for inline variables D127233 <https://reviews.llvm.org/D127233>. To provide this guarantee, we also need to emit DeferredDeclsToEmit in DFS order. e5df59ff78faebd897e81907606ce6074aac0df originally supported this but it is not exactly DFS order for cases like the one in cwg362. For the example of Fib<5>, it triggers instantiation of Fib<4> and Fib<3>. However, due to the way GlobalEagerInstantiationScope is implemented, Fib<4> does not actually trigger Fib<3> instantiation since it is already triggered by Fib<5>. This breaks the guarantee. This patch makes sure DeferredDeclsToEmit is emit in DFS order by moving DeferredDeclsToEmit storage from the call stack to a explicit stack-like data structure. Then the DFS order could be enforced. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127259 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp
Index: clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp =================================================================== --- clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp +++ clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp @@ -29,10 +29,34 @@ // ALL: @_ZN1AIbE1aE ={{.*}} global i32 10 template<> int A<bool>::a = 10; -// ALL: @llvm.global_ctors = appending global [8 x { i32, void ()*, i8* }] +// ALL: @llvm.global_ctors = appending global [16 x { i32, void ()*, i8* }] -// ELF: [{ i32, void ()*, i8* } { i32 65535, void ()* @[[unordered1:[^,]*]], i8* bitcast (i32* @_ZN1AIsE1aE to i8*) }, -// MACHO: [{ i32, void ()*, i8* } { i32 65535, void ()* @[[unordered1:[^,]*]], i8* null }, +// ELF: [{ i32, void ()*, i8* } { i32 65535, void ()* @[[unordered10:[^,]*]], i8* bitcast (i32* @_ZN3FibILi2EE1aE to i8*) }, +// MACHO: [{ i32, void ()*, i8* } { i32 65535, void ()* @[[unordered10:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered9:[^,]*]], i8* bitcast (i32* @_ZN3FibILi3EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered9:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered11:[^,]*]], i8* bitcast (i32* @_ZN3FibILi4EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered11:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered8:[^,]*]], i8* bitcast (i32* @_ZN3FibILi5EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered8:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered14:[^,]*]], i8* bitcast (i32* @_ZN4Fib2ILi2EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered14:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered15:[^,]*]], i8* bitcast (i32* @_ZN4Fib2ILi3EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered15:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered13:[^,]*]], i8* bitcast (i32* @_ZN4Fib2ILi4EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered13:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered12:[^,]*]], i8* bitcast (i32* @_ZN4Fib2ILi5EE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered12:[^,]*]], i8* null }, + +// ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered1:[^,]*]], i8* bitcast (i32* @_ZN1AIsE1aE to i8*) }, +// MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered1:[^,]*]], i8* null }, // ELF: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered2:[^,]*]], i8* bitcast (i16* @_Z1xIsE to i8*) }, // MACHO: { i32, void ()*, i8* } { i32 65535, void ()* @[[unordered2:[^,]*]], i8* null }, @@ -54,7 +78,7 @@ // ALL: { i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_static_member_variable_explicit_specialization.cpp, i8* null }] /// llvm.used ensures SHT_INIT_ARRAY in a section group cannot be GCed. -// ELF: @llvm.used = appending global [6 x i8*] [i8* bitcast (i32* @_ZN1AIsE1aE to i8*), i8* bitcast (i16* @_Z1xIsE to i8*), i8* bitcast (i32* @_ZN2ns1aIiE1iE to i8*), i8* bitcast (i32* @_ZN2ns1b1iIiEE to i8*), i8* bitcast (i32* @_ZN1AIvE1aE to i8*), i8* @_Z1xIcE] +// ELF: @llvm.used = appending global [14 x i8*] [i8* bitcast (i32* @_ZN1AIsE1aE to i8*), i8* bitcast (i16* @_Z1xIsE to i8*), i8* bitcast (i32* @_ZN2ns1aIiE1iE to i8*), i8* bitcast (i32* @_ZN2ns1b1iIiEE to i8*), i8* bitcast (i32* @_ZN1AIvE1aE to i8*), i8* @_Z1xIcE, i8* bitcast (i32* @_ZN3FibILi5EE1aE to i8*), i8* bitcast (i32* @_ZN3FibILi3EE1aE to i8*), i8* bitcast (i32* @_ZN3FibILi2EE1aE to i8*), i8* bitcast (i32* @_ZN3FibILi4EE1aE to i8*), i8* bitcast (i32* @_ZN4Fib2ILi5EE1aE to i8*), i8* bitcast (i32* @_ZN4Fib2ILi4EE1aE to i8*), i8* bitcast (i32* @_ZN4Fib2ILi2EE1aE to i8*), i8* bitcast (i32* @_ZN4Fib2ILi3EE1aE to i8*)] template int A<short>::a; // Unordered int b = foo(); @@ -94,6 +118,18 @@ } int *use_internal_a = &Internal<int>::a; +template<int n> struct Fib { static int a; }; +template<> int Fib<0>::a = 0; +template<> int Fib<1>::a = 1; +template<int n> int Fib<n>::a = Fib<n-2>::a + Fib<n-1>::a; +int f = Fib<5>::a; + +template<int n> struct Fib2 { static int a; }; +template<> int Fib2<0>::a = 0; +template<> int Fib2<1>::a = 1; +template<int n> int Fib2<n>::a = Fib2<n-1>::a + Fib2<n-2>::a; +int f2 = Fib2<5>::a; + #endif // ALL: define internal void @[[unordered1]]( @@ -131,6 +167,38 @@ // ALL: store {{.*}} @_ZN12_GLOBAL__N_18InternalIiE1aE // ALL: ret +// ALL: define internal void @[[unordered8]]( +// ALL: store {{.*}} @_ZN3FibILi5EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered9]]( +// ALL: store {{.*}} @_ZN3FibILi3EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered10]]( +// ALL: store {{.*}} @_ZN3FibILi2EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered11]]( +// ALL: store {{.*}} @_ZN3FibILi4EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered12]]( +// ALL: store {{.*}} @_ZN4Fib2ILi5EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered13]]( +// ALL: store {{.*}} @_ZN4Fib2ILi4EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered14]]( +// ALL: store {{.*}} @_ZN4Fib2ILi2EE1aE +// ALL: ret + +// ALL: define internal void @[[unordered15]]( +// ALL: store {{.*}} @_ZN4Fib2ILi3EE1aE +// ALL: ret + // ALL: define internal void @_GLOBAL__sub_I_static_member_variable_explicit_specialization.cpp() // We call unique stubs for every ordered dynamic initializer in the TU. // ALL: call @@ -141,5 +209,7 @@ // ALL: call // ALL: call // ALL: call +// ALL: call +// ALL: call // ALL-NOT: call // ALL: ret Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -19375,6 +19375,18 @@ // multiple times. SemaRef.PendingInstantiations .push_back(std::make_pair(Var, PointOfInstantiation)); + } else { + for (auto &I : SemaRef.SavedPendingInstantiations) { + auto Iter = llvm::find_if( + I, [Var](const Sema::PendingImplicitInstantiation &P) { + return P.first == Var; + }); + if (Iter != I.end()) { + SemaRef.PendingInstantiations.push_back(*Iter); + I.erase(Iter); + break; + } + } } } } Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -9397,14 +9397,21 @@ /// eagerly. SmallVector<PendingImplicitInstantiation, 1> LateParsedInstantiations; + SmallVector<SmallVector<VTableUse, 16>, 8> SavedVTableUses; + SmallVector<std::deque<PendingImplicitInstantiation>, 8> + SavedPendingInstantiations; + class GlobalEagerInstantiationScope { public: GlobalEagerInstantiationScope(Sema &S, bool Enabled) : S(S), Enabled(Enabled) { if (!Enabled) return; - SavedPendingInstantiations.swap(S.PendingInstantiations); - SavedVTableUses.swap(S.VTableUses); + S.SavedPendingInstantiations.emplace_back(); + S.SavedPendingInstantiations.back().swap(S.PendingInstantiations); + + S.SavedVTableUses.emplace_back(); + S.SavedVTableUses.back().swap(S.VTableUses); } void perform() { @@ -9420,26 +9427,28 @@ // Restore the set of pending vtables. assert(S.VTableUses.empty() && "VTableUses should be empty before it is discarded."); - S.VTableUses.swap(SavedVTableUses); + S.VTableUses.swap(S.SavedVTableUses.back()); + S.SavedVTableUses.pop_back(); // Restore the set of pending implicit instantiations. if (S.TUKind != TU_Prefix || !S.LangOpts.PCHInstantiateTemplates) { assert(S.PendingInstantiations.empty() && "PendingInstantiations should be empty before it is discarded."); - S.PendingInstantiations.swap(SavedPendingInstantiations); + S.PendingInstantiations.swap(S.SavedPendingInstantiations.back()); + S.SavedPendingInstantiations.pop_back(); } else { // Template instantiations in the PCH may be delayed until the TU. - S.PendingInstantiations.swap(SavedPendingInstantiations); - S.PendingInstantiations.insert(S.PendingInstantiations.end(), - SavedPendingInstantiations.begin(), - SavedPendingInstantiations.end()); + S.PendingInstantiations.swap(S.SavedPendingInstantiations.back()); + S.PendingInstantiations.insert( + S.PendingInstantiations.end(), + S.SavedPendingInstantiations.back().begin(), + S.SavedPendingInstantiations.back().end()); + S.SavedPendingInstantiations.pop_back(); } } private: Sema &S; - SmallVector<VTableUse, 16> SavedVTableUses; - std::deque<PendingImplicitInstantiation> SavedPendingInstantiations; bool Enabled; };
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits