[Clang] perform "maximum TLS alignment" check for template instantiation

[ychen]

Summary

follow up https://github.com/llvm/llvm-project/commit/d30e2eefc3cf8dfd2210aefd62f13a6e7c011b43

[mizvekov]

clang/lib/Sema/SemaDecl.cpp

14038–14039

Is this needed? It seems getMaxTLSAlign() will return 0 in that case anyway.

[ychen]

• remove unnecessary check

[ychen]

clang/lib/Sema/SemaDecl.cpp

14038–14039

It is checked in the other place where err_tls_var_aligned_over_maximum is used so I put it here. I'll remove the check here and there.

clang/include/clang/Sema/Sema.h

public:
   void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
   void ActOnCXXForRangeDecl(Decl *D);
   StmtResult ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,
                                         IdentifierInfo *Ident,
                                         ParsedAttributes &Attrs);
   void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
   void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);
   void CheckStaticLocalForDllExport(VarDecl *VD);
+  void CheckThreadLocalForLargeAlignment(VarDecl *VD);
   void FinalizeDeclaration(Decl *D);
   DeclGroupPtrTy FinalizeDeclaratorGroup(Scope *S, const DeclSpec &DS,
                                          ArrayRef<Decl *> Group);
   DeclGroupPtrTy BuildDeclaratorGroup(MutableArrayRef<Decl *> Group);
 
   /// Should be called on all declarations that might have attached
   /// documentation comments.
   void ActOnDocumentableDecl(Decl *D);

clang/lib/Sema/SemaDecl.cpp

void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
 
   } else if (Attr *A = FD->getAttr<DLLImportStaticLocalAttr>()) {
     auto *NewAttr = DLLImportAttr::CreateImplicit(getASTContext(), *A);
     NewAttr->setInherited(true);
     VD->addAttr(NewAttr);
   }
 }
 
+void Sema::CheckThreadLocalForLargeAlignment(VarDecl *VD) {
+  assert(VD->getTLSKind());
+
+  // Perform TLS alignment check here after attributes attached to the variable
+  // which may affect the alignment have been processed. Only perform the check
+  // if the target has a maximum TLS alignment (zero means no constraints).
+  if (unsigned MaxAlign = Context.getTargetInfo().getMaxTLSAlign()) {
+    // Protect the check so that it's not performed on dependent types and
+    // dependent alignments (we can't determine the alignment in that case).
+    if (!VD->hasDependentAlignment()) {
+      CharUnits MaxAlignChars = Context.toCharUnitsFromBits(MaxAlign);
+      if (Context.getDeclAlign(VD) > MaxAlignChars) {
+        Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
+            << (unsigned)Context.getDeclAlign(VD).getQuantity() << VD
+            << (unsigned)MaxAlignChars.getQuantity();
+      }
+    }
+  }
+}
+
 /// FinalizeDeclaration - called by ParseDeclarationAfterDeclarator to perform
 /// any semantic actions necessary after any initializer has been attached.
 void Sema::FinalizeDeclaration(Decl *ThisDecl) {
   // Note that we are no longer parsing the initializer for this declaration.
   ParsingInitForAutoVars.erase(ThisDecl);
 
   VarDecl *VD = dyn_cast_or_null<VarDecl>(ThisDecl);
   if (!VD)
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
   if (auto *DD = dyn_cast<DecompositionDecl>(ThisDecl)) {
     for (auto *BD : DD->bindings()) {
       FinalizeDeclaration(BD);
     }
   }
 
   checkAttributesAfterMerging(*this, *VD);
 
-  // Perform TLS alignment check here after attributes attached to the variable
-  // which may affect the alignment have been processed. Only perform the check
-  // if the target has a maximum TLS alignment (zero means no constraints).
-  if (unsigned MaxAlign = Context.getTargetInfo().getMaxTLSAlign()) {
-    // Protect the check so that it's not performed on dependent types and
-    // dependent alignments (we can't determine the alignment in that case).
-    if (VD->getTLSKind() && !VD->hasDependentAlignment()) {
-      CharUnits MaxAlignChars = Context.toCharUnitsFromBits(MaxAlign);
-      if (Context.getDeclAlign(VD) > MaxAlignChars) {
-        Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
-          << (unsigned)Context.getDeclAlign(VD).getQuantity() << VD
-          << (unsigned)MaxAlignChars.getQuantity();
-      }
-    }
-  }
-
   if (VD->isStaticLocal())
     CheckStaticLocalForDllExport(VD);
 
+  if (VD->getTLSKind())
+    CheckThreadLocalForLargeAlignment(VD);
+
   // Perform check for initializers of device-side global variables.
   // CUDA allows empty constructors as initializers (see E.2.3.1, CUDA
   // 7.5). We must also apply the same checks to all __shared__
   // variables whether they are local or not. CUDA also allows
   // constant initializers for __constant__ and __device__ variables.
   if (getLangOpts().CUDA)
     checkAllowedCUDAInitializer(VD);
 

clang/lib/Sema/SemaDeclAttr.cpp

if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
     MaximumAlignment = std::min(MaximumAlignment, uint64_t(8192));
   if (AlignVal > MaximumAlignment) {
     Diag(AttrLoc, diag::err_attribute_aligned_too_great)
         << MaximumAlignment << E->getSourceRange();
     return;
   }
 
   const auto *VD = dyn_cast<VarDecl>(D);
-  if (VD && Context.getTargetInfo().isTLSSupported()) {
+  if (VD) {
     unsigned MaxTLSAlign =
         Context.toCharUnitsFromBits(Context.getTargetInfo().getMaxTLSAlign())
             .getQuantity();
     if (MaxTLSAlign && AlignVal > MaxTLSAlign &&
         VD->getTLSKind() != VarDecl::TLS_None) {
       Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
           << (unsigned)AlignVal << VD << MaxTLSAlign;
       return;

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

if (D->isNRVOVariable() && !Var->isInvalidDecl()) {
     Var->setNRVOVariable(NRVO);
   }
 
   Var->setImplicit(D->isImplicit());
 
   if (Var->isStaticLocal())
     SemaRef.CheckStaticLocalForDllExport(Var);
 
+  if (Var->getTLSKind())
+    SemaRef.CheckThreadLocalForLargeAlignment(Var);
+
   return Var;
 }
 
 Decl *TemplateDeclInstantiator::VisitAccessSpecDecl(AccessSpecDecl *D) {
   AccessSpecDecl* AD
     = AccessSpecDecl::Create(SemaRef.Context, D->getAccess(), Owner,
                              D->getAccessSpecifierLoc(), D->getColonLoc());
   Owner->addHiddenDecl(AD);

clang/test/Sema/tls_alignment.cpp

 
 int baz ()
 {
     return foo.some_data[0] + bar.some_data[1] + bar2.some_data[2] +
            bar3.some_aligned_data[3] + bar4.some_data[4] +
            bar5.some_data[5];
 }
 
-
-// Verify alignment check where a dependent type is involved.
-// The check is (correctly) not performed on "t", but the check still is
-// performed on the structure as a whole once it has been instantiated.
-
 template<class T> struct templated_tls {
     static __thread T t;
     T other_t __attribute__(( aligned(64) ));
 };
  __thread templated_tls<int> blah; // expected-error{{alignment (64) of thread-local variable}}
 
-int blag() {
-    return blah.other_t * 2;
-}
-
-
-// Verify alignment check where the alignment is a template parameter.
-// The check is only performed during instantiation.
 template <int N>
 struct S {
+  struct alignas(64) B {};
+  struct alignas(N) C {};
+  static inline void f() {
+    thread_local B b; // expected-error{{alignment (64) of thread-local variable}}
+    thread_local C c; // expected-error{{alignment (64) of thread-local variable}}
+  }
+  template<int J> static inline thread_local int b alignas(J) = J; // expected-error{{alignment (64) of thread-local variable}}
   static int __thread __attribute__((aligned(N))) x; // expected-error{{alignment (64) of thread-local variable}}
 };
 
+int blag() {
+    // Verify alignment check where the alignment is a template parameter.
+    // The check is only performed during instantiation.
     S<64> s_instance; // expected-note{{in instantiation of template class 'S<64>' requested here}}
+
+    // Verify alignment for dependent local variables.
+    S<64>::f(); // expected-note{{in instantiation of member function 'S<64>::f' requested here}}
+
+    // Verify alignment check where a dependent type is involved.
+    // The check is (correctly) not performed on "t", but the check still is
+    // performed on the structure as a whole once it has been instantiated.
+    return blah.other_t * 2 + S<64>::b<64>; // expected-note{{in instantiation of static data member 'S<64>::b' requested here}}
+}