From 39287e759f9a2a53050d70b9b34b4be14f09d1e3 Mon Sep 17 00:00:00 2001 From: Shoaib Meenai Date: Wed, 9 Jan 2019 20:05:16 +0000 Subject: [PATCH] [CodeGen] Clarify comment about COFF common symbol alignment After a discussion on the commit thread, it seems the 32 byte alignment limitation is an MSVC toolchain artifact, not an inherent COFF restriction. Clarify the comment accordingly, since saying COFF in the comment but using isKnownWindowsMSVCEnvironment in the conditional is confusing. Also add a newline before the comment, which is consistent with the local style. Differential Revision: https://reviews.llvm.org/D56466 llvm-svn: 350754 --- clang/lib/CodeGen/CodeGenModule.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index ab0ac67d8e8d..244738042cef 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3761,8 +3761,12 @@ static bool isVarDeclStrongDefinition(const ASTContext &Context, } } } - // COFF doesn't support alignments greater than 32, so these cannot be - // in common. + + // Microsoft's link.exe doesn't support alignments greater than 32 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 (Context.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment() && Context.getTypeAlignIfKnown(D->getType()) > 32) return true;