[clang] [CIR] Upstream global variable linkage types (PR #129072)

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 28 16:44:17 PST 2025


================
@@ -210,6 +223,193 @@ void CIRGenModule::emitGlobalDefinition(clang::GlobalDecl gd,
   llvm_unreachable("Invalid argument to CIRGenModule::emitGlobalDefinition");
 }
 
+static bool shouldBeInCOMDAT(CIRGenModule &cgm, const Decl &d) {
+  assert(!cir::MissingFeatures::supportComdat());
+
+  if (d.hasAttr<SelectAnyAttr>())
+    return true;
+
+  GVALinkage linkage;
+  if (auto *vd = dyn_cast<VarDecl>(&d))
+    linkage = cgm.getASTContext().GetGVALinkageForVariable(vd);
+  else
+    linkage =
+        cgm.getASTContext().GetGVALinkageForFunction(cast<FunctionDecl>(&d));
+
+  switch (linkage) {
+  case clang::GVA_Internal:
+  case clang::GVA_AvailableExternally:
+  case clang::GVA_StrongExternal:
+    return false;
+  case clang::GVA_DiscardableODR:
+  case clang::GVA_StrongODR:
+    return true;
+  }
+  llvm_unreachable("No such linkage");
+}
+
+// TODO(CIR): this could be a common method between LLVM codegen.
+static bool isVarDeclStrongDefinition(const ASTContext &astContext,
+                                      CIRGenModule &cgm, const VarDecl *vd,
+                                      bool noCommon) {
+  // Don't give variables common linkage if -fno-common was specified unless it
+  // was overridden by a NoCommon attribute.
+  if ((noCommon || vd->hasAttr<NoCommonAttr>()) && !vd->hasAttr<CommonAttr>())
+    return true;
+
+  // C11 6.9.2/2:
+  //   A declaration of an identifier for an object that has file scope without
+  //   an initializer, and without a storage-class specifier or with the
+  //   storage-class specifier static, constitutes a tentative definition.
+  if (vd->getInit() || vd->hasExternalStorage())
+    return true;
+
+  // A variable cannot be both common and exist in a section.
+  if (vd->hasAttr<SectionAttr>())
+    return true;
+
+  // A variable cannot be both common and exist in a section.
+  // We don't try to determine which is the right section in the front-end.
+  // If no specialized section name is applicable, it will resort to default.
+  if (vd->hasAttr<PragmaClangBSSSectionAttr>() ||
+      vd->hasAttr<PragmaClangDataSectionAttr>() ||
+      vd->hasAttr<PragmaClangRelroSectionAttr>() ||
+      vd->hasAttr<PragmaClangRodataSectionAttr>())
+    return true;
+
+  // Thread local vars aren't considered common linkage.
+  if (vd->getTLSKind())
+    return true;
+
+  // Tentative definitions marked with WeakImportAttr are true definitions.
+  if (vd->hasAttr<WeakImportAttr>())
+    return true;
+
+  // A variable cannot be both common and exist in a comdat.
+  if (shouldBeInCOMDAT(cgm, *vd))
+    return true;
+
+  // Declarations with a required alignment do not have common linkage in MSVC
+  // mode.
+  if (astContext.getTargetInfo().getCXXABI().isMicrosoft()) {
+    if (vd->hasAttr<AlignedAttr>())
+      return true;
+    QualType varType = vd->getType();
+    if (astContext.isAlignmentRequired(varType))
+      return true;
+
+    if (const auto *rt = varType->getAs<RecordType>()) {
+      const RecordDecl *rd = rt->getDecl();
+      for (const FieldDecl *fd : rd->fields()) {
+        if (fd->isBitField())
+          continue;
+        if (fd->hasAttr<AlignedAttr>())
+          return true;
+        if (astContext.isAlignmentRequired(fd->getType()))
+          return true;
+      }
+    }
+  }
+
+  // Microsoft's link.exe doesn't support alignments greater than 32 bytes for
+  // common symbols, so symbols with greater alignment requirements cannot be
+  // common.
+  // Other COFF linkers (ld.bfd and LLD) support arbitrary power-of-two
+  // alignments for common symbols via the aligncomm directive, so this
+  // restriction only applies to MSVC environments.
+  if (astContext.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment() &&
+      astContext.getTypeAlignIfKnown(vd->getType()) >
+          astContext.toBits(CharUnits::fromQuantity(32)))
+    return true;
+
+  return false;
+}
+
+cir::GlobalLinkageKind CIRGenModule::getCIRLinkageForDeclarator(
----------------
bcardosolopes wrote:

> this would require changes to the incubator and CodeGen first, as some functions are private methods or rely on langOpts from CodeGen. So this is more of a question for @erichkeane and @bcardosolopes—would this be feasible?

I'm not a big fan of depending on LLVM stuff at the CIR level, given we can get all info we need from the AST and even though it'd be a practical solution - my take is to keep that at the lowering level.

> This is likely a general policy question: how tightly should CodeGens be coupled, and should we modify main CodeGen to better support API reuse for CIRCodeGen? I expect many similar copies, independent of the actual CIR and MLIR, to appear during upstreaming.

I believe reuse is always great, but IMO I don't think we need to generate extra work for upstreaming on anything other than things like a `ASTCodegenHelpers` atm.

https://github.com/llvm/llvm-project/pull/129072


More information about the cfe-commits mailing list