smeenai created this revision. smeenai added reviewers: erichkeane, majnemer, mstorsjo.
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. Repository: rC Clang https://reviews.llvm.org/D56466 Files: lib/CodeGen/CodeGenModule.cpp Index: lib/CodeGen/CodeGenModule.cpp =================================================================== --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -3761,8 +3761,11 @@ } } } - // COFF doesn't support alignments greater than 32, so these cannot be - // in common. + + // MSVC doesn't support alignments greater than 32 for common symbols, so + // symbols with greater alignment requirements cannot be common. Non-MSVC COFF + // environments support arbitrary power-of-two alignments for common symbols + // via the aligncomm directive, so the restriction doesn't apply there. if (Context.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment() && Context.getTypeAlignIfKnown(D->getType()) > 32) return true;
Index: lib/CodeGen/CodeGenModule.cpp =================================================================== --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -3761,8 +3761,11 @@ } } } - // COFF doesn't support alignments greater than 32, so these cannot be - // in common. + + // MSVC doesn't support alignments greater than 32 for common symbols, so + // symbols with greater alignment requirements cannot be common. Non-MSVC COFF + // environments support arbitrary power-of-two alignments for common symbols + // via the aligncomm directive, so the restriction doesn't apply there. if (Context.getTargetInfo().getTriple().isKnownWindowsMSVCEnvironment() && Context.getTypeAlignIfKnown(D->getType()) > 32) return true;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits