[clang] e423885 - [CodeGen] guarantee templated static variables are initialized in the reverse instantiation order

Yuanfang Chen via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 3 00:13:00 PST 2023


Author: Yuanfang Chen
Date: 2023-03-03T00:12:28-08:00
New Revision: e423885e272c0e57c6d240d07bc934e0a8649417

URL: https://github.com/llvm/llvm-project/commit/e423885e272c0e57c6d240d07bc934e0a8649417
DIFF: https://github.com/llvm/llvm-project/commit/e423885e272c0e57c6d240d07bc934e0a8649417.diff

LOG: [CodeGen] guarantee templated static variables are initialized in the reverse instantiation order

Based on Richard's suggestion in 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 variables are passed to CodeGen.

This is based on the same ordering guarantee needed for inline variables D127233.
To provide this guarantee, we also need to
emit DeferredDeclsToEmit in the DFS order. https://github.com/llvm/llvm-project/commit/e5df59ff78faebd897e81907606ce6074aac0df6
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 the 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 emitted in DFS order by moving
DeferredDeclsToEmit storage from the call stack to an explicit stack-like
data structure. Then the DFS order could be enforced.

Differential Revision: https://reviews.llvm.org/D127259

Added: 
    

Modified: 
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaExpr.cpp
    clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 60fed8603ec55..d1342876f85dc 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9847,14 +9847,21 @@ class Sema final {
   /// 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() {
@@ -9870,26 +9877,28 @@ class Sema final {
       // 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;
   };
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3207f42aad63c..c3e2b6fdeaf1f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19897,6 +19897,18 @@ static void DoMarkVarDeclReferenced(
         // 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;
+          }
+        }
       }
     }
   }

diff  --git a/clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp b/clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp
index de6e86fe56aa1..46c4c4d391769 100644
--- a/clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp
+++ b/clang/test/CodeGenCXX/static-member-variable-explicit-specialization.cpp
@@ -29,32 +29,56 @@ template<> int A<char>::a;
 // ALL: @_ZN1AIbE1aE ={{.*}} global i32 10
 template<> int A<bool>::a = 10;
 
-// ALL: @llvm.global_ctors = appending global [8 x { i32, ptr, ptr }]
+// ALL: @llvm.global_ctors = appending global [16 x { i32, ptr, ptr }]
 
-// ELF: [{ i32, ptr, ptr } { i32 65535, ptr @[[unordered1:[^,]*]], ptr @_ZN1AIsE1aE },
-// MACHO: [{ i32, ptr, ptr } { i32 65535, ptr @[[unordered1:[^,]*]], ptr null },
+// ELF:  [{ i32, ptr, ptr } { i32 65535, ptr @[[unordered:[^,]*]], ptr @_ZN1AIsE1aE },
+// MACHO: [{ i32, ptr, ptr } { i32 65535, ptr @[[unordered:[^,]*]], ptr null },
 
-// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered2:[^,]*]], ptr @_Z1xIsE },
-// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered2:[^,]*]], ptr null },
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered7:[^,]*]], ptr @_Z1xIsE },
+// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered7:[^,]*]], ptr null },
 
-// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered3:[^,]*]], ptr @_ZN2ns1aIiE1iE },
-// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered3:[^,]*]], ptr null },
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered10:[^,]*]], ptr @_ZN2ns1aIiE1iE },
+// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered10:[^,]*]], ptr null },
 
-// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered4:[^,]*]], ptr @_ZN2ns1b1iIiEE },
-// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered4:[^,]*]], ptr null },
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered11:[^,]*]], ptr @_ZN2ns1b1iIiEE },
+// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered11:[^,]*]], ptr null },
 
-// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered5:[^,]*]], ptr @_ZN1AIvE1aE },
-// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered5:[^,]*]], ptr null },
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered14:[^,]*]], ptr @_ZN1AIvE1aE },
+// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered14:[^,]*]], ptr null },
 
-// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered6:[^,]*]], ptr @_Z1xIcE },
-// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered6:[^,]*]], ptr null },
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered15:[^,]*]], ptr @_Z1xIcE },
+// MACHO:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered15:[^,]*]], ptr null },
 
-// ALL:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered7:[^,]*]], ptr null },
+// ALL:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered16:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered19:[^,]*]], ptr @_ZN3FibILi2EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered19:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered18:[^,]*]], ptr @_ZN3FibILi3EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered18:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered20:[^,]*]], ptr @_ZN3FibILi4EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered20:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered17:[^,]*]], ptr @_ZN3FibILi5EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered17:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered23:[^,]*]], ptr @_ZN4Fib2ILi2EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered23:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered24:[^,]*]], ptr @_ZN4Fib2ILi3EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered24:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered22:[^,]*]], ptr @_ZN4Fib2ILi4EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered22:[^,]*]], ptr null },
+
+// ELF:  { i32, ptr, ptr } { i32 65535, ptr @[[unordered21:[^,]*]], ptr @_ZN4Fib2ILi5EE1aE },
+// MACHO: { i32, ptr, ptr } { i32 65535, ptr @[[unordered21:[^,]*]], ptr null }, 
 
 // ALL:  { i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I_static_member_variable_explicit_specialization.cpp, ptr null }]
 
 /// llvm.used ensures SHT_INIT_ARRAY in a section group cannot be GCed.
-// ELF: @llvm.used = appending global [6 x ptr] [ptr @_ZN1AIsE1aE, ptr @_Z1xIsE, ptr @_ZN2ns1aIiE1iE, ptr @_ZN2ns1b1iIiEE, ptr @_ZN1AIvE1aE, ptr @_Z1xIcE]
+// ELF: @llvm.used = appending global [14 x ptr] [ptr @_ZN1AIsE1aE, ptr @_Z1xIsE, ptr @_ZN2ns1aIiE1iE, ptr @_ZN2ns1b1iIiEE, ptr @_ZN1AIvE1aE, ptr @_Z1xIcE, ptr @_ZN3FibILi5EE1aE, ptr @_ZN3FibILi3EE1aE, ptr @_ZN3FibILi2EE1aE, ptr @_ZN3FibILi4EE1aE, ptr @_ZN4Fib2ILi5EE1aE, ptr @_ZN4Fib2ILi4EE1aE, ptr @_ZN4Fib2ILi2EE1aE, ptr @_ZN4Fib2ILi3EE1aE]
 
 template int A<short>::a;  // Unordered
 int b = foo();
@@ -94,43 +118,82 @@ template<typename T> int Internal<T>::a = foo();
 }
 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]](
+// ALL: define internal void @[[unordered]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_ZN1AIsE1aE
 // ALL: ret
 
-// ALL: define internal void @[[unordered2]](
+// ALL: define internal void @[[unordered7]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_Z1xIsE
 // ALL: ret
 
-// ALL: define internal void @[[unordered3]](
+// ALL: define internal void @[[unordered10]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_ZN2ns1aIiE1iE
 // ALL: ret
 
-// ALL: define internal void @[[unordered4]](
+// ALL: define internal void @[[unordered11]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_ZN2ns1b1iIiEE
 // ALL: ret
 
-// ALL: define internal void @[[unordered5]](
-// ALL: call i32 @foo()
-// ALL: store {{.*}} @_ZN1AIvE1aE
-// ALL: ret
-
-// ALL: define internal void @[[unordered6]](
+// ALL: define internal void @[[unordered15]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_Z1xIcE
 // ALL: ret
 
-// ALL: define internal void @[[unordered7]](
+// ALL: define internal void @[[unordered16]](
 // ALL: call i32 @foo()
 // ALL: store {{.*}} @_ZN12_GLOBAL__N_18InternalIiE1aE
 // ALL: ret
 
+// ALL: define internal void @[[unordered17]](
+// ALL: store {{.*}} @_ZN3FibILi5EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered18]](
+// ALL: store {{.*}} @_ZN3FibILi3EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered19]](
+// ALL: store {{.*}} @_ZN3FibILi2EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered20]](
+// ALL: store {{.*}} @_ZN3FibILi4EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered21]](
+// ALL: store {{.*}} @_ZN4Fib2ILi5EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered22]](
+// ALL: store {{.*}} @_ZN4Fib2ILi4EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered23]](
+// ALL: store {{.*}} @_ZN4Fib2ILi2EE1aE
+// ALL: ret
+
+// ALL: define internal void @[[unordered24]](
+// 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 +204,7 @@ int *use_internal_a = &Internal<int>::a;
 // ALL: call
 // ALL: call
 // ALL: call
+// ALL: call
+// ALL: call
 // ALL-NOT: call
 // ALL: ret


        


More information about the cfe-commits mailing list