llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (adams381) <details> <summary>Changes</summary> Implement emitDeclInvariant to emit llvm.invariant.start intrinsic for global variables with constant storage. This enables optimizations by marking when a global becomes read-only after initialization. ## Changes - Add emitDeclInvariant and emitInvariantStart functions in CIRGenCXX.cpp - Add emitInvariantStart declaration in CIRGenFunction.h - Update emitCXXGlobalVarDeclInit to call emitDeclInvariant for constant storage globals after initialization - Update getOrCreateCIRGlobal to set constant flag on globals with constant storage - Add comprehensive test covering positive and negative cases ## Implementation Details The implementation handles address spaces correctly, dynamically constructing the intrinsic name (e.g., invariant.start.p0, invariant.start.p10) based on the pointer's address space. The intrinsic is only emitted when optimizations are enabled, matching classic codegen behavior. ## Testing All tests pass (411/412, 1 unsupported). The test file includes CIR, LLVM, and OGCG checks for both optimized and non-optimized builds. --- Full diff: https://github.com/llvm/llvm-project/pull/171915.diff 4 Files Affected: - (modified) clang/lib/CIR/CodeGen/CIRGenCXX.cpp (+67-5) - (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+2) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+17-8) - (added) clang/test/CIR/CodeGen/global-constant-storage.cpp (+244) ``````````diff diff --git a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp index 71568ec87a31b..c0890f6b42663 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXX.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCXX.cpp @@ -16,11 +16,54 @@ #include "clang/AST/GlobalDecl.h" #include "clang/CIR/MissingFeatures.h" +#include "llvm/ADT/SmallString.h" #include "llvm/Support/SaveAndRestore.h" +#include "llvm/Support/raw_ostream.h" using namespace clang; using namespace clang::CIRGen; +/// Emit code to cause the variable at the given address to be considered as +/// constant from this point onwards. +static void emitDeclInvariant(CIRGenFunction &CGF, const VarDecl *D) { + mlir::Value addr = CGF.cgm.getAddrOfGlobalVar(D); + CGF.emitInvariantStart(CGF.getContext().getTypeSizeInChars(D->getType()), + addr, CGF.getLoc(D->getSourceRange())); +} + +void CIRGenFunction::emitInvariantStart(CharUnits Size, mlir::Value Addr, + mlir::Location loc) { + // Do not emit the intrinsic if we're not optimizing. + if (!cgm.getCodeGenOpts().OptimizationLevel) + return; + + CIRGenBuilderTy &builder = getBuilder(); + + // Create the size constant as i64 + uint64_t width = Size.getQuantity(); + mlir::Value sizeValue = builder.getConstInt(loc, builder.getSInt64Ty(), + static_cast<int64_t>(width)); + + // Determine address space for intrinsic name + unsigned addrSpace = 0; + if (auto ptrTy = mlir::dyn_cast<cir::PointerType>(Addr.getType())) + addrSpace = + ptrTy.getAddrSpace() ? ptrTy.getAddrSpace().getValue().getUInt() : 0; + + // Format intrinsic name with address space suffix (e.g., + // "invariant.start.p0", "invariant.start.p10") + llvm::SmallString<32> intrinsicName; + llvm::raw_svector_ostream os(intrinsicName); + os << "invariant.start.p" << addrSpace; + + // Create the intrinsic call. The llvm.invariant.start intrinsic returns a + // token, but we don't need to capture it. The return type is set to match + // the address type for consistency with the operation signature. + cir::LLVMIntrinsicCallOp::create( + builder, loc, builder.getStringAttr(intrinsicName), Addr.getType(), + mlir::ValueRange{sizeValue, Addr}); +} + static void emitDeclInit(CIRGenFunction &cgf, const VarDecl *varDecl, cir::GlobalOp globalOp) { assert((varDecl->hasGlobalStorage() || @@ -234,13 +277,32 @@ void CIRGenModule::emitCXXGlobalVarDeclInit(const VarDecl *varDecl, bool needsDtor = varDecl->needsDestruction(getASTContext()) == QualType::DK_cxx_destructor; + bool isConstantStorage = + varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor); // PerformInit, constant store invariant / destroy handled below. - if (performInit) + if (performInit) { emitDeclInit(cgf, varDecl, addr); - - if (varDecl->getType().isConstantStorage(getASTContext(), true, !needsDtor)) - errorNYI(varDecl->getSourceRange(), "global with constant storage"); - else + // For constant storage, emit invariant.start in the ctor region after + // initialization but before the yield. + if (isConstantStorage) { + CIRGenBuilderTy &builder = cgf.getBuilder(); + mlir::OpBuilder::InsertionGuard guard(builder); + // Set insertion point to end of ctor region (before yield) + if (!addr.getCtorRegion().empty()) { + mlir::Block *block = &addr.getCtorRegion().back(); + // Find the yield op and insert before it + mlir::Operation *yieldOp = block->getTerminator(); + if (yieldOp) { + builder.setInsertionPoint(yieldOp); + emitDeclInvariant(cgf, varDecl); + } + } + } + } else if (isConstantStorage) { + emitDeclInvariant(cgf, varDecl); + } + + if (!isConstantStorage) emitDeclDestroy(cgf, varDecl, addr); return; } diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index efe0fe5fcc979..627698c5b19af 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -1611,6 +1611,8 @@ class CIRGenFunction : public CIRGenTypeCache { mlir::Value emitRuntimeCall(mlir::Location loc, cir::FuncOp callee, llvm::ArrayRef<mlir::Value> args = {}); + void emitInvariantStart(CharUnits Size, mlir::Value Addr, mlir::Location loc); + /// Emit the computation of the specified expression of scalar type. mlir::Value emitScalarExpr(const clang::Expr *e, bool ignoreResultAssign = false); diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 1ad1c2fa41aa1..715e877fcca3d 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -652,10 +652,19 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty, mlir::Location loc = getLoc(d->getSourceRange()); + // Calculate constant storage flag before creating the global. + bool isConstant = false; + if (d) { + bool needsDtor = + d->needsDestruction(astContext) == QualType::DK_cxx_destructor; + isConstant = d->getType().isConstantStorage( + astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/!needsDtor); + } + // mlir::SymbolTable::Visibility::Public is the default, no need to explicitly // mark it as such. cir::GlobalOp gv = - CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, false, + CIRGenModule::createGlobalOp(*this, loc, mangledName, ty, isConstant, /*insertPoint=*/entry.getOperation()); // This is the first use or definition of a mangled name. If there is a @@ -675,10 +684,6 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty, errorNYI(d->getSourceRange(), "OpenMP target global variable"); gv.setAlignmentAttr(getSize(astContext.getDeclAlign(d))); - // FIXME: This code is overly simple and should be merged with other global - // handling. - gv.setConstant(d->getType().isConstantStorage( - astContext, /*ExcludeCtor=*/false, /*ExcludeDtor=*/false)); setLinkageForGV(gv, d); @@ -870,10 +875,14 @@ void CIRGenModule::emitGlobalVarDefinition(const clang::VarDecl *vd, emitter->finalize(gv); // If it is safe to mark the global 'constant', do so now. + // Use the same logic as emitCXXGlobalVarDeclInit to determine constant + // storage. + bool needsDtor = + vd->needsDestruction(astContext) == QualType::DK_cxx_destructor; gv.setConstant((vd->hasAttr<CUDAConstantAttr>() && langOpts.CUDAIsDevice) || - (!needsGlobalCtor && !needsGlobalDtor && - vd->getType().isConstantStorage( - astContext, /*ExcludeCtor=*/true, /*ExcludeDtor=*/true))); + vd->getType().isConstantStorage(astContext, + /*ExcludeCtor=*/true, + /*ExcludeDtor=*/!needsDtor)); assert(!cir::MissingFeatures::opGlobalSection()); // Set CIR's linkage type as appropriate. diff --git a/clang/test/CIR/CodeGen/global-constant-storage.cpp b/clang/test/CIR/CodeGen/global-constant-storage.cpp new file mode 100644 index 0000000000000..8aa240cf8fc5b --- /dev/null +++ b/clang/test/CIR/CodeGen/global-constant-storage.cpp @@ -0,0 +1,244 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t-cir.ll +// RUN: FileCheck --check-prefix=LLVM --input-file=%t-cir.ll %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=OGCG --input-file=%t.ll %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt.ll +// RUN: FileCheck --check-prefix=LLVM-OPT --input-file=%t-opt.ll %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu -emit-llvm -O1 -disable-llvm-passes %s -o %t-opt-ogcg.ll +// RUN: FileCheck --check-prefix=OGCG-OPT --input-file=%t-opt-ogcg.ll %s + +// Test for global with constant storage - const object with constructor but no destructor +// Check that we add an llvm.invariant.start to mark when a global becomes read-only. + +struct A { + A(); + int n; +}; + +// Should emit invariant.start - has constructor, no destructor, no mutable +extern const A a = A(); + +struct A2 { + A2(); + constexpr ~A2() {} + int n; +}; + +// Should emit invariant.start - constexpr destructor doesn't prevent constant storage +extern const A2 a2 = A2(); + +struct B { + B(); + mutable int n; +}; + +// Should NOT emit invariant.start - has mutable member +extern const B b = B(); + +struct CWithDtor { + CWithDtor(); + ~CWithDtor(); + int n; +}; + +// Should NOT emit invariant.start - has non-constexpr destructor +extern const CWithDtor c_with_dtor = CWithDtor(); + +// Simple case - just const C c; (no initializer) - Andy's suggestion +class C { +public: + C(); + int a; + int b; +}; + +const C c; + +// CIR checks for 'a' - should have constant storage +// CIR: cir.global constant external @a = #cir.zero : !rec_A +// CIR: cir.func internal private @__cxx_global_var_init() { +// CIR: %[[OBJ:.*]] = cir.get_global @a : !cir.ptr<!rec_A> +// CIR: cir.call @_ZN1AC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A>) -> () +// CIR: cir.return +// CIR: } + +// CIR checks for 'a2' - should have constant storage (constexpr dtor) +// CIR: cir.global constant external @a2 = #cir.zero : !rec_A2 +// CIR: cir.func internal private @__cxx_global_var_init.1() { +// CIR: %[[OBJ:.*]] = cir.get_global @a2 : !cir.ptr<!rec_A2> +// CIR: cir.call @_ZN2A2C1Ev(%[[OBJ]]) : (!cir.ptr<!rec_A2>) -> () +// CIR: cir.return +// CIR: } + +// CIR checks for 'b' - should NOT have constant storage (mutable member) +// CIR: cir.global external @b = #cir.zero : !rec_B +// CIR: cir.func internal private @__cxx_global_var_init.2() { +// CIR: %[[OBJ:.*]] = cir.get_global @b : !cir.ptr<!rec_B> +// CIR: cir.call @_ZN1BC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_B>) -> () +// CIR: cir.return +// CIR: } + +// CIR checks for 'c_with_dtor' - should NOT have constant storage (non-constexpr dtor) +// CIR: cir.global external @c_with_dtor = #cir.zero : !rec_CWithDtor +// CIR: cir.func internal private @__cxx_global_var_init.3() { +// CIR: %[[OBJ:.*]] = cir.get_global @c_with_dtor : !cir.ptr<!rec_CWithDtor> +// CIR: cir.call @_ZN9CWithDtorC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_CWithDtor>) -> () +// CIR: cir.return +// CIR: } + +// CIR checks for 'c' - Andy's simple case, should have constant storage (internal linkage) +// CIR: cir.global {{.*}} constant internal {{.*}} @_ZL1c = #cir.zero : !rec_C +// CIR: cir.func internal private @__cxx_global_var_init.4() { +// CIR: %[[OBJ:.*]] = cir.get_global @_ZL1c : !cir.ptr<!rec_C> +// CIR: cir.call @_ZN1CC1Ev(%[[OBJ]]) : (!cir.ptr<!rec_C>) -> () +// CIR: cir.return +// CIR: } + +// LLVM checks (no optimization) +// Check all globals first (they appear at the top) +// LLVM: @a ={{.*}} constant {{.*}} zeroinitializer +// LLVM: @a2 ={{.*}} constant {{.*}} zeroinitializer +// LLVM: @b ={{.*}} global {{.*}} zeroinitializer +// LLVM: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer +// LLVM: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer + +// Then check the init functions +// LLVM: define internal void @__cxx_global_var_init() { +// LLVM: call void @_ZN1AC1Ev(ptr @a) +// LLVM: ret void +// LLVM: } + +// LLVM: define internal void @__cxx_global_var_init.1() { +// LLVM: call void @_ZN2A2C1Ev(ptr @a2) +// LLVM: ret void +// LLVM: } + +// LLVM: define internal void @__cxx_global_var_init.2() { +// LLVM: call void @_ZN1BC1Ev(ptr @b) +// LLVM: ret void +// LLVM: } + +// LLVM: define internal void @__cxx_global_var_init.3() { +// LLVM: call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor) +// LLVM: ret void +// LLVM: } + +// LLVM: define internal void @__cxx_global_var_init.4() { +// LLVM: call void @_ZN1CC1Ev(ptr @_ZL1c) +// LLVM: ret void +// LLVM: } + +// OGCG checks (no optimization) +// Check all globals first (they appear at the top) +// OGCG: @a ={{.*}} global {{.*}} zeroinitializer +// OGCG: @a2 ={{.*}} global {{.*}} zeroinitializer +// OGCG: @b ={{.*}} global {{.*}} zeroinitializer +// OGCG: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer +// OGCG: @_ZL1c ={{.*}} global {{.*}} zeroinitializer + +// Then check the init functions +// OGCG: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" { +// OGCG: call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a) +// OGCG: ret void +// OGCG: } + +// OGCG: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" { +// OGCG: call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2) +// OGCG: ret void +// OGCG: } + +// OGCG: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" { +// OGCG: call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b) +// OGCG: ret void +// OGCG: } + +// OGCG: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" { +// OGCG: call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor) +// OGCG: ret void +// OGCG: } + +// OGCG: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" { +// OGCG: call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c) +// OGCG: ret void +// OGCG: } + +// With optimization enabled, should emit invariant.start intrinsic for constant storage cases +// Check all globals first (they appear at the top) +// LLVM-OPT: @a ={{.*}} constant {{.*}} zeroinitializer +// LLVM-OPT: @a2 ={{.*}} constant {{.*}} zeroinitializer +// LLVM-OPT: @b ={{.*}} global {{.*}} zeroinitializer +// LLVM-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer +// LLVM-OPT: @_ZL1c ={{.*}} constant {{.*}} zeroinitializer + +// Then check the init functions with invariant.start +// LLVM-OPT: define internal void @__cxx_global_var_init() { +// LLVM-OPT: call void @_ZN1AC1Ev(ptr @a) +// LLVM-OPT: call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a) +// LLVM-OPT: ret void +// LLVM-OPT: } + +// LLVM-OPT: define internal void @__cxx_global_var_init.1() { +// LLVM-OPT: call void @_ZN2A2C1Ev(ptr @a2) +// LLVM-OPT: call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2) +// LLVM-OPT: ret void +// LLVM-OPT: } + +// LLVM-OPT: define internal void @__cxx_global_var_init.2() { +// LLVM-OPT: call void @_ZN1BC1Ev(ptr @b) +// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b) +// LLVM-OPT: ret void +// LLVM-OPT: } + +// LLVM-OPT: define internal void @__cxx_global_var_init.3() { +// LLVM-OPT: call void @_ZN9CWithDtorC1Ev(ptr @c_with_dtor) +// LLVM-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor) +// LLVM-OPT: ret void +// LLVM-OPT: } + +// LLVM-OPT: define internal void @__cxx_global_var_init.4() { +// LLVM-OPT: call void @_ZN1CC1Ev(ptr @_ZL1c) +// LLVM-OPT: call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c) +// LLVM-OPT: ret void +// LLVM-OPT: } + +// OGCG-OPT checks (with optimization) +// Check all globals first (they appear at the top) +// OGCG-OPT: @a ={{.*}} global {{.*}} zeroinitializer +// OGCG-OPT: @a2 ={{.*}} global {{.*}} zeroinitializer +// OGCG-OPT: @b ={{.*}} global {{.*}} zeroinitializer +// OGCG-OPT: @c_with_dtor ={{.*}} global {{.*}} zeroinitializer +// OGCG-OPT: @_ZL1c ={{.*}} global {{.*}} zeroinitializer + +// Then check the init functions with invariant.start +// OGCG-OPT: define internal void @__cxx_global_var_init() {{.*}} section ".text.startup" { +// OGCG-OPT: call void @_ZN1AC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a) +// OGCG-OPT: call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a) +// OGCG-OPT: ret void +// OGCG-OPT: } + +// OGCG-OPT: define internal void @__cxx_global_var_init.1() {{.*}} section ".text.startup" { +// OGCG-OPT: call void @_ZN2A2C1Ev(ptr noundef nonnull align 4 dereferenceable(4) @a2) +// OGCG-OPT: call {{.*}}@llvm.invariant.start.p0(i64 4, ptr @a2) +// OGCG-OPT: ret void +// OGCG-OPT: } + +// OGCG-OPT: define internal void @__cxx_global_var_init.2() {{.*}} section ".text.startup" { +// OGCG-OPT: call void @_ZN1BC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @b) +// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @b) +// OGCG-OPT: ret void +// OGCG-OPT: } + +// OGCG-OPT: define internal void @__cxx_global_var_init.3() {{.*}} section ".text.startup" { +// OGCG-OPT: call void @_ZN9CWithDtorC1Ev(ptr noundef nonnull align 4 dereferenceable(4) @c_with_dtor) +// OGCG-OPT-NOT: call {{.*}}@llvm.invariant.start.p0(i64 {{.*}}, ptr @c_with_dtor) +// OGCG-OPT: ret void +// OGCG-OPT: } + +// OGCG-OPT: define internal void @__cxx_global_var_init.4() {{.*}} section ".text.startup" { +// OGCG-OPT: call void @_ZN1CC1Ev(ptr noundef nonnull align 4 dereferenceable(8) @_ZL1c) +// OGCG-OPT: call {{.*}}@llvm.invariant.start.p0(i64 8, ptr @_ZL1c) +// OGCG-OPT: ret void +// OGCG-OPT: } + `````````` </details> https://github.com/llvm/llvm-project/pull/171915 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
