dblaikie updated this revision to Diff 550097. dblaikie added a comment. Diagnose specific reasons
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156726/new/ https://reviews.llvm.org/D156726 Files: clang/include/clang/AST/Type.h clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/AST/Type.cpp clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/CGDeclCXX.cpp clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprConstant.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/Targets/AMDGPU.cpp clang/lib/Sema/SemaDecl.cpp clang/test/CodeGenCXX/sections.cpp clang/test/SemaCXX/attr-section.cpp
Index: clang/test/SemaCXX/attr-section.cpp =================================================================== --- clang/test/SemaCXX/attr-section.cpp +++ clang/test/SemaCXX/attr-section.cpp @@ -48,3 +48,24 @@ const int const_global_var __attribute__((section("template_fn1"))) = 42; // expected-error {{'const_global_var' causes a section type conflict with 'template_fn1'}} int mut_global_var __attribute__((section("template_fn2"))) = 42; // expected-note {{declared here}} template <typename> __attribute__((section("template_fn2"))) void template_fn2() {} // expected-error {{'template_fn2' causes a section type conflict with 'mut_global_var'}} + +namespace mutable_member { +struct t1 { + mutable int i; +}; +extern const t1 v1; +__attribute__((section("mutable_member"))) const t1 v1{}; +extern int i; +__attribute__((section("mutable_member"))) int i{}; +} // namespace mutable_member + +namespace non_trivial_ctor { +struct t1 { + t1(); + constexpr t1(int) { } +}; +extern const t1 v1; +__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{declared here}} +extern const t1 v2; +__attribute__((section("non_trivial_ctor"))) const t1 v2{3}; // expected-error {{'v2' causes a section type conflict with 'v1'}} +} // namespace non_trivial_ctor Index: clang/test/CodeGenCXX/sections.cpp =================================================================== --- clang/test/CodeGenCXX/sections.cpp +++ clang/test/CodeGenCXX/sections.cpp @@ -1,12 +1,36 @@ -// RUN: %clang_cc1 -emit-llvm -triple i686-pc-win32 -fms-extensions -o - %s | FileCheck %s +// RUN: %clang_cc1 -emit-llvm -triple i686-pc-win32 -fms-extensions -verify -o - %s | FileCheck %s extern "C" { +struct Mutable { +mutable int i = 3; +}; +extern const Mutable mutable_default_section; +const Mutable mutable_default_section; +struct Normal { + int i = 2; +}; +extern const Normal normal_default_section; +const Normal normal_default_section; #pragma const_seg(".my_const") #pragma bss_seg(".my_bss") int D = 1; #pragma data_seg(".data") int a = 1; +extern const Mutable mutable_custom_section; +const Mutable mutable_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'mutable_custom_section' due to the presence of a mutable field}} +extern const Normal normal_custom_section; +const Normal normal_custom_section; +struct NonTrivialDtor { + ~NonTrivialDtor(); +}; +extern const NonTrivialDtor non_trivial_dtor_custom_section; +const NonTrivialDtor non_trivial_dtor_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'non_trivial_dtor_custom_section' due to the presence of a non-trivial destructor}} +struct NonTrivialCtor { + NonTrivialCtor(); +}; +extern const NonTrivialCtor non_trivial_ctor_custom_section; +const NonTrivialCtor non_trivial_ctor_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'non_trivial_ctor_custom_section' due to the presence of a non-trivial constructor}} #pragma data_seg(push, label, ".data2") extern const int b; const int b = 1; @@ -74,10 +98,19 @@ #pragma section("short_section", short) // Pragma section ignores "short". __declspec(allocate("short_section")) short short_var = 42; + +struct t2 { t2(); }; +extern const t2 non_trivial_ctor; +__declspec(allocate("non_trivial_ctor_section")) const t2 non_trivial_ctor_var; } + +//CHECK: @mutable_default_section = dso_local global %struct.Mutable { i32 3 }, align 4{{$}} +//CHECK: @normal_default_section = dso_local constant %struct.Normal { i32 2 }, align 4{{$}} //CHECK: @D = dso_local global i32 1 //CHECK: @a = dso_local global i32 1, section ".data" +//CHECK: @mutable_custom_section = dso_local global %struct.Mutable { i32 3 }, section ".data", align 4 +//CHECK: @normal_custom_section = dso_local constant %struct.Normal { i32 2 }, section ".my_const", align 4 //CHECK: @b = dso_local constant i32 1, section ".my_const" //CHECK: @[[MYSTR:.*]] = {{.*}} unnamed_addr constant [11 x i8] c"my string!\00" //CHECK: @s = dso_local global ptr @[[MYSTR]], section ".data2" @@ -96,6 +129,7 @@ //CHECK: @implicitly_read_write = dso_local global i32 42, section "no_section_attributes" //CHECK: @long_var = dso_local global i32 42, section "long_section" //CHECK: @short_var = dso_local global i16 42, section "short_section" +//CHECK: @non_trivial_ctor_var = internal global %struct.t2 zeroinitializer, section "non_trivial_ctor_section" //CHECK: define dso_local void @g() //CHECK: define dso_local void @h() {{.*}} section ".my_code" //CHECK: define dso_local void @h2() {{.*}} section ".my_code" Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -14360,25 +14360,33 @@ !inTemplateInstantiation()) { PragmaStack<StringLiteral *> *Stack = nullptr; int SectionFlags = ASTContext::PSF_Read; - if (var->getType().isConstQualified()) { - if (HasConstInit) - Stack = &ConstSegStack; - else { - Stack = &BSSSegStack; - SectionFlags |= ASTContext::PSF_Write; - } - } else if (var->hasInit() && HasConstInit) { - Stack = &DataSegStack; - SectionFlags |= ASTContext::PSF_Write; + bool MSVCEnv = + Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment(); + std::optional<QualType::NonConstantStorageReason> Reason; + if (var->hasInit() && HasConstInit && !(Reason = + var->getType().isNonConstantStorage(Context, true, false))) { + Stack = &ConstSegStack; } else { - Stack = &BSSSegStack; SectionFlags |= ASTContext::PSF_Write; + Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack; } if (const SectionAttr *SA = var->getAttr<SectionAttr>()) { if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec) SectionFlags |= ASTContext::PSF_Implicit; UnifySection(SA->getName(), SectionFlags, var); } else if (Stack->CurrentValue) { + if (Stack != &ConstSegStack && MSVCEnv && + ConstSegStack.CurrentValue != ConstSegStack.DefaultValue && + var->getType().isConstQualified()) { + assert(!Reason || + Reason != QualType::NonConstantStorageReason:: + NonConstNonReferenceType && + "This case should've already been handled elsewhere"); + Diag(var->getLocation(), diag::warn_section_msvc_compat) + << var << ConstSegStack.CurrentValue << (int)(!HasConstInit + ? QualType::NonConstantStorageReason::NonTrivialCtor + : *Reason); + } SectionFlags |= ASTContext::PSF_Implicit; auto SectionName = Stack->CurrentValue->getString(); var->addAttr(SectionAttr::CreateImplicit(Context, SectionName, Index: clang/lib/CodeGen/Targets/AMDGPU.cpp =================================================================== --- clang/lib/CodeGen/Targets/AMDGPU.cpp +++ clang/lib/CodeGen/Targets/AMDGPU.cpp @@ -418,7 +418,7 @@ return AddrSpace; // Only promote to address space 4 if VarDecl has constant initialization. - if (CGM.isTypeConstant(D->getType(), false, false) && + if (D->getType().isConstantStorage(CGM.getContext(), false, false) && D->hasConstantInitialization()) { if (auto ConstAS = CGM.getTarget().getConstantAddressSpace()) return *ConstAS; Index: clang/lib/CodeGen/CodeGenModule.h =================================================================== --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -814,8 +814,6 @@ return getTBAAAccessInfo(AccessType); } - bool isTypeConstant(QualType QTy, bool ExcludeCtor, bool ExcludeDtor); - bool isPaddedAtomicType(QualType type); bool isPaddedAtomicType(const AtomicType *type); Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -3377,7 +3377,7 @@ // codegen for global variables, because they may be marked as threadprivate. if (LangOpts.OpenMP && LangOpts.OpenMPUseTLS && getContext().getTargetInfo().isTLSSupported() && isa<VarDecl>(Global) && - !isTypeConstant(Global->getType(), false, false) && + !Global->getType().isConstantStorage(getContext(), false, false) && !OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Global)) return false; @@ -4581,27 +4581,6 @@ return {FTy, C}; } -/// isTypeConstant - Determine whether an object of this type can be emitted -/// as a constant. -/// -/// If ExcludeCtor is true, the duration when the object's constructor runs -/// will not be considered. The caller will need to verify that the object is -/// not written to during its construction. ExcludeDtor works similarly. -bool CodeGenModule::isTypeConstant(QualType Ty, bool ExcludeCtor, - bool ExcludeDtor) { - if (!Ty.isConstant(Context) && !Ty->isReferenceType()) - return false; - - if (Context.getLangOpts().CPlusPlus) { - if (const CXXRecordDecl *Record - = Context.getBaseElementType(Ty)->getAsCXXRecordDecl()) - return ExcludeCtor && !Record->hasMutableFields() && - (Record->hasTrivialDestructor() || ExcludeDtor); - } - - return true; -} - /// GetOrCreateLLVMGlobal - If the specified mangled name is not in the module, /// create and return an llvm GlobalVariable with the specified type and address /// space. If there is something in the module with the specified name, return @@ -4709,7 +4688,7 @@ // FIXME: This code is overly simple and should be merged with other global // handling. - GV->setConstant(isTypeConstant(D->getType(), false, false)); + GV->setConstant(D->getType().isConstantStorage(getContext(), false, false)); GV->setAlignment(getContext().getDeclAlign(D).getAsAlign()); @@ -5270,7 +5249,7 @@ // If it is safe to mark the global 'constant', do so now. GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor && - isTypeConstant(D->getType(), true, true)); + D->getType().isConstantStorage(getContext(), true, true)); // If it is in a read-only section, mark it 'constant'. if (const SectionAttr *SA = D->getAttr<SectionAttr>()) { @@ -6342,8 +6321,9 @@ emitter.emplace(*this); InitialValue = emitter->emitForInitializer(*Value, AddrSpace, MaterializedType); - Constant = isTypeConstant(MaterializedType, /*ExcludeCtor*/ Value, - /*ExcludeDtor*/ false); + Constant = + MaterializedType.isConstantStorage(getContext(), /*ExcludeCtor*/ Value, + /*ExcludeDtor*/ false); Type = InitialValue->getType(); } else { // No initializer, the initialization will be provided when we Index: clang/lib/CodeGen/CGExprConstant.cpp =================================================================== --- clang/lib/CodeGen/CGExprConstant.cpp +++ clang/lib/CodeGen/CGExprConstant.cpp @@ -934,7 +934,7 @@ auto GV = new llvm::GlobalVariable( CGM.getModule(), C->getType(), - CGM.isTypeConstant(E->getType(), true, false), + E->getType().isConstantStorage(CGM.getContext(), true, false), llvm::GlobalValue::InternalLinkage, C, ".compoundliteral", nullptr, llvm::GlobalVariable::NotThreadLocal, CGM.getContext().getTargetAddressSpace(addressSpace)); Index: clang/lib/CodeGen/CGExpr.cpp =================================================================== --- clang/lib/CodeGen/CGExpr.cpp +++ clang/lib/CodeGen/CGExpr.cpp @@ -391,7 +391,7 @@ QualType Ty = Inner->getType(); if (CGF.CGM.getCodeGenOpts().MergeAllConstants && (Ty->isArrayType() || Ty->isRecordType()) && - CGF.CGM.isTypeConstant(Ty, true, false)) + Ty.isConstantStorage(CGF.getContext(), true, false)) if (auto Init = ConstantEmitter(CGF).tryEmitAbstract(Inner, Ty)) { auto AS = CGF.CGM.GetGlobalConstantAddressSpace(); auto *GV = new llvm::GlobalVariable( Index: clang/lib/CodeGen/CGDeclCXX.cpp =================================================================== --- clang/lib/CodeGen/CGDeclCXX.cpp +++ clang/lib/CodeGen/CGDeclCXX.cpp @@ -217,7 +217,7 @@ D.needsDestruction(getContext()) == QualType::DK_cxx_destructor; if (PerformInit) EmitDeclInit(*this, D, DeclAddr); - if (CGM.isTypeConstant(D.getType(), true, !NeedsDtor)) + if (D.getType().isConstantStorage(getContext(), true, !NeedsDtor)) EmitDeclInvariant(*this, D, DeclPtr); else EmitDeclDestroy(*this, D, DeclAddr); Index: clang/lib/CodeGen/CGDecl.cpp =================================================================== --- clang/lib/CodeGen/CGDecl.cpp +++ clang/lib/CodeGen/CGDecl.cpp @@ -398,7 +398,8 @@ bool NeedsDtor = D.needsDestruction(getContext()) == QualType::DK_cxx_destructor; - GV->setConstant(CGM.isTypeConstant(D.getType(), true, !NeedsDtor)); + GV->setConstant( + D.getType().isConstantStorage(getContext(), true, !NeedsDtor)); GV->setInitializer(Init); emitter.finalize(GV); @@ -1498,7 +1499,8 @@ if ((!getLangOpts().OpenCL || Ty.getAddressSpace() == LangAS::opencl_constant) && (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && - !isEscapingByRef && CGM.isTypeConstant(Ty, true, !NeedsDtor))) { + !isEscapingByRef && + Ty.isConstantStorage(getContext(), true, !NeedsDtor))) { EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage); // Signal this condition to later callbacks. Index: clang/lib/AST/Type.cpp =================================================================== --- clang/lib/AST/Type.cpp +++ clang/lib/AST/Type.cpp @@ -112,6 +112,25 @@ return T.getAddressSpace() == LangAS::opencl_constant; } +std::optional<QualType::NonConstantStorageReason> +QualType::isNonConstantStorage(const ASTContext &Ctx, bool ExcludeCtor, + bool ExcludeDtor) { + if (!isConstant(Ctx) && !(*this)->isReferenceType()) + return NonConstantStorageReason::NonConstNonReferenceType; + if (!Ctx.getLangOpts().CPlusPlus) + return std::nullopt; + if (const CXXRecordDecl *Record = + Ctx.getBaseElementType(*this)->getAsCXXRecordDecl()) { + if (!ExcludeCtor) + return NonConstantStorageReason::NonTrivialCtor; + if (Record->hasMutableFields()) + return NonConstantStorageReason::MutableField; + if (!Record->hasTrivialDestructor() && !ExcludeDtor) + return NonConstantStorageReason::NonTrivialDtor; + } + return std::nullopt; +} + // C++ [temp.dep.type]p1: // A type is dependent if it is... // - an array type constructed from any dependent type or whose Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -996,6 +996,9 @@ def err_pragma_pack_identifer_not_supported : Error< "specifying an identifier within `#pragma pack` is not supported on this target">; def err_section_conflict : Error<"%0 causes a section type conflict with %1">; +def warn_section_msvc_compat : Warning<"`#pragma const_seg` for section %1 will" + " not apply to %0 due to the presence of a %select{mutable field||non-trivial constructor|non-trivial destructor}2">, + InGroup<IncompatibleMSPragmaSection>; def err_no_base_classes : Error<"invalid use of '__super', %0 has no base classes">; def err_invalid_super_scope : Error<"invalid use of '__super', " "this keyword can only be used inside class or member function scope">; Index: clang/include/clang/Basic/DiagnosticGroups.td =================================================================== --- clang/include/clang/Basic/DiagnosticGroups.td +++ clang/include/clang/Basic/DiagnosticGroups.td @@ -446,6 +446,7 @@ def : DiagGroup<"import">; def GNUIncludeNext : DiagGroup<"gnu-include-next">; def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">; +def IncompatibleMSPragmaSection : DiagGroup<"incompatible-ms-pragma-section">; def IncompatiblePointerTypesDiscardsQualifiers : DiagGroup<"incompatible-pointer-types-discards-qualifiers">; def IncompatibleFunctionPointerTypes Index: clang/include/clang/AST/Type.h =================================================================== --- clang/include/clang/AST/Type.h +++ clang/include/clang/AST/Type.h @@ -817,6 +817,26 @@ /// Determine whether this type is const-qualified. bool isConstQualified() const; + enum class NonConstantStorageReason { + MutableField, + NonConstNonReferenceType, + NonTrivialCtor, + NonTrivialDtor, + }; + /// Determine whether instances of this type can be placed in immutable + /// storage. + /// If ExcludeCtor is true, the duration when the object's constructor runs + /// will not be considered. The caller will need to verify that the object is + /// not written to during its construction. ExcludeDtor works similarly. + std::optional<NonConstantStorageReason> + isNonConstantStorage(const ASTContext &Ctx, bool ExcludeCtor, + bool ExcludeDtor); + + bool isConstantStorage(const ASTContext &Ctx, bool ExcludeCtor, + bool ExcludeDtor) { + return !isNonConstantStorage(Ctx, ExcludeCtor, ExcludeDtor); + } + /// Determine whether this particular QualType instance has the /// "restrict" qualifier set, without looking through typedefs that may have /// added "restrict" at a different level.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits