https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/197314
>From 1247c9ea470473f33033ec959ab4922935079d5f Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Tue, 12 May 2026 13:19:13 -0700 Subject: [PATCH] [CIR] Lower materialize-global-temporaries that self-reference This showed up in a test suite, but cases where a materialized global temporary references itself, we emitted an NYI. This patch implements it by creating a 'throw-away' global op that can be referenced by name, then erased/thrown away. Since it is referenced by name in global-view, all of the uses I could reproduce didn't require a 'replaceAllUsesWith', but the call is left in case some sort of initialization causes a reference directly to the global. Also, as a drive-by, `createGlobalOp` was a static member function that took the CIRGenModule as an argument for some reason, so this patch just makes it a member function. --- clang/lib/CIR/CodeGen/CIRGenCUDANV.cpp | 4 +- clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp | 9 +-- clang/lib/CIR/CodeGen/CIRGenModule.cpp | 79 +++++++++++-------- clang/lib/CIR/CodeGen/CIRGenModule.h | 6 +- .../test/CIR/CodeGen/self-ref-temporaries.cpp | 32 ++++++++ 5 files changed, 86 insertions(+), 44 deletions(-) create mode 100644 clang/test/CIR/CodeGen/self-ref-temporaries.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenCUDANV.cpp b/clang/lib/CIR/CodeGen/CIRGenCUDANV.cpp index 0466c212fe165..868e80e2a2b31 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCUDANV.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCUDANV.cpp @@ -334,8 +334,8 @@ mlir::Operation *CIRGenNVCUDARuntime::getKernelHandle(cir::FuncOp fn, StringRef globalName = cgm.getMangledName( gd.getWithKernelReferenceKind(KernelReferenceKind::Kernel)); cir::PointerType fnPtrTy = builder.getPointerTo(fn.getFunctionType()); - cir::GlobalOp globalOp = CIRGenModule::createGlobalOp( - cgm, fn.getLoc(), globalName, fnPtrTy, /*isConstant=*/true); + cir::GlobalOp globalOp = + cgm.createGlobalOp(fn.getLoc(), globalName, fnPtrTy, /*isConstant=*/true); globalOp->setAttr("alignment", builder.getI64IntegerAttr( cgm.getPointerAlign().getQuantity())); diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp index 750bb7b87c10d..560a4de9b9b01 100644 --- a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp @@ -1202,8 +1202,8 @@ CIRGenItaniumRTTIBuilder::getAddrOfExternalRTTIDescriptor(mlir::Location loc, // From LLVM codegen => Note for the future: If we would ever like to do // deferred emission of RTTI, check if emitting vtables opportunistically // need any adjustment. - gv = CIRGenModule::createGlobalOp(cgm, loc, name, builder.getUInt8PtrTy(), - /*isConstant=*/true); + gv = cgm.createGlobalOp(loc, name, builder.getUInt8PtrTy(), + /*isConstant=*/true); const CXXRecordDecl *rd = ty->getAsCXXRecordDecl(); cgm.setGVProperties(gv, rd); @@ -1601,9 +1601,8 @@ mlir::Attribute CIRGenItaniumRTTIBuilder::buildTypeInfo( // Create new global and search for an existing global. auto oldGV = dyn_cast_or_null<cir::GlobalOp>(cgm.getGlobalValue(name)); - cir::GlobalOp gv = - CIRGenModule::createGlobalOp(cgm, loc, name, init.getType(), - /*isConstant=*/true); + cir::GlobalOp gv = cgm.createGlobalOp(loc, name, init.getType(), + /*isConstant=*/true); gv.setLinkage(linkage); // Export the typeinfo in the same circumstances as the vtable is diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp index 9ebdac56006a4..555be9ed890fe 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp @@ -683,12 +683,12 @@ mlir::Operation *CIRGenModule::getGlobalValue(StringRef name) { } cir::GlobalOp -CIRGenModule::createGlobalOp(CIRGenModule &cgm, mlir::Location loc, - StringRef name, mlir::Type t, bool isConstant, +CIRGenModule::createGlobalOp(mlir::Location loc, StringRef name, mlir::Type t, + bool isConstant, mlir::ptr::MemorySpaceAttrInterface addrSpace, mlir::Operation *insertPoint) { cir::GlobalOp g; - CIRGenBuilderTy &builder = cgm.getBuilder(); + CIRGenBuilderTy &builder = getBuilder(); { mlir::OpBuilder::InsertionGuard guard(builder); @@ -700,22 +700,22 @@ CIRGenModule::createGlobalOp(CIRGenModule &cgm, mlir::Location loc, builder.setInsertionPoint(insertPoint); } else { // Group global operations together at the top of the module. - if (cgm.lastGlobalOp) - builder.setInsertionPointAfter(cgm.lastGlobalOp); + if (lastGlobalOp) + builder.setInsertionPointAfter(lastGlobalOp); else - builder.setInsertionPointToStart(cgm.getModule().getBody()); + builder.setInsertionPointToStart(getModule().getBody()); } g = cir::GlobalOp::create(builder, loc, name, t, isConstant, addrSpace); if (!insertPoint) - cgm.lastGlobalOp = g; + lastGlobalOp = g; // Default to private until we can judge based on the initializer, // since MLIR doesn't allow public declarations. mlir::SymbolTable::setSymbolVisibility( g, mlir::SymbolTable::Visibility::Private); } - cgm.symbolLookupCache[g.getSymNameAttr()] = g; + symbolLookupCache[g.getSymNameAttr()] = g; return g; } @@ -1130,9 +1130,8 @@ CIRGenModule::getOrCreateCIRGlobal(StringRef mangledName, mlir::Type ty, // mlir::SymbolTable::Visibility::Public is the default, no need to explicitly // mark it as such. - cir::GlobalOp gv = CIRGenModule::createGlobalOp( - *this, loc, mangledName, ty, isConstant, declCIRAS, - /*insertPoint=*/entry.getOperation()); + cir::GlobalOp gv = createGlobalOp(loc, mangledName, ty, isConstant, declCIRAS, + /*insertPoint=*/entry.getOperation()); // If we already created a global with the same mangled name (but different // type) before, remove it from its parent. @@ -1298,8 +1297,8 @@ static void emitUsed(CIRGenModule &cgm, StringRef name, cir::ConstArrayAttr initAttr = cir::ConstArrayAttr::get( arrayTy, mlir::ArrayAttr::get(&cgm.getMLIRContext(), usedArray)); - cir::GlobalOp gv = CIRGenModule::createGlobalOp(cgm, loc, name, arrayTy, - /*isConstant=*/false); + cir::GlobalOp gv = cgm.createGlobalOp(loc, name, arrayTy, + /*isConstant=*/false); gv.setLinkage(cir::GlobalLinkageKind::AppendingLinkage); gv.setInitialValueAttr(initAttr); gv.setSectionAttr(builder.getStringAttr("llvm.metadata")); @@ -1731,7 +1730,7 @@ cir::GlobalOp CIRGenModule::createOrReplaceCXXRuntimeVariable( } // Create a new variable. - gv = createGlobalOp(*this, loc, name, ty); + gv = createGlobalOp(loc, name, ty); // Set up extra information and add to the module gv.setLinkageAttr( @@ -2033,8 +2032,8 @@ generateStringLiteral(mlir::Location loc, mlir::TypedAttr c, // Create a global variable for this string // FIXME(cir): check for insertion point in module level. - cir::GlobalOp gv = CIRGenModule::createGlobalOp( - cgm, loc, globalName, c.getType(), !cgm.getLangOpts().WritableStrings); + cir::GlobalOp gv = cgm.createGlobalOp(loc, globalName, c.getType(), + !cgm.getLangOpts().WritableStrings); // Set up extra information and add to the module gv.setAlignmentAttr(cgm.getSize(alignment)); @@ -3409,7 +3408,7 @@ void CIRGenModule::release() { cir::LangAddressSpaceAttr::get(&getMLIRContext(), getGlobalVarAddressSpace(nullptr)); - auto gv = createGlobalOp(*this, loc, cuidName, int8Ty, + auto gv = createGlobalOp(loc, cuidName, int8Ty, /*isConstant=*/false, addrSpace); gv.setLinkage(cir::GlobalLinkageKind::ExternalLinkage); // Initialize with zero @@ -3492,14 +3491,13 @@ void CIRGenModule::emitAliasDefinition(GlobalDecl gd) { // TODO(cir): Make GlobalAlias a separate op. cir::CIRGlobalValueInterface alias = - isFunction - ? mlir::cast<cir::CIRGlobalValueInterface>( - createCIRFunction(loc, mangledName, - mlir::cast<cir::FuncType>(declTy), - cast<FunctionDecl>(d)) - .getOperation()) - : mlir::cast<cir::CIRGlobalValueInterface>( - createGlobalOp(*this, loc, mangledName, declTy).getOperation()); + isFunction ? mlir::cast<cir::CIRGlobalValueInterface>( + createCIRFunction(loc, mangledName, + mlir::cast<cir::FuncType>(declTy), + cast<FunctionDecl>(d)) + .getOperation()) + : mlir::cast<cir::CIRGlobalValueInterface>( + createGlobalOp(loc, mangledName, declTy).getOperation()); alias.setAliasee(aa->getAliasee()); alias.setLinkage(linkage); mlir::SymbolTable::setSymbolVisibility(alias, visibility); @@ -3697,10 +3695,7 @@ CIRGenModule::getAddrOfGlobalTemporary(const MaterializeTemporaryExpr *mte, materializedType = mte->getType(); CharUnits align = getASTContext().getTypeAlignInChars(materializedType); - - auto insertResult = materializedGlobalTemporaryMap.insert({mte, nullptr}); - if (!insertResult.second) - errorNYI(mte->getSourceRange(), "duplicate materialized temporaries"); + mlir::Location loc = getLoc(mte->getSourceRange()); // FIXME: If an externally-visible declaration extends multiple temporaries, // we need to give each temporary the same name in every translation unit (and @@ -3710,6 +3705,20 @@ CIRGenModule::getAddrOfGlobalTemporary(const MaterializeTemporaryExpr *mte, getCXXABI().getMangleContext().mangleReferenceTemporary( varDecl, mte->getManglingNumber(), out); + auto insertResult = materializedGlobalTemporaryMap.insert({mte, nullptr}); + if (!insertResult.second) { + mlir::Type type = getTypes().convertTypeForMem(materializedType); + // We've seen this before: either we already created it or we're in the + // process of doing so. + if (!insertResult.first->second) { + // We recursively re-entered this function, probably during emission of + // the initializer. Create a placeholder. + insertResult.first->second = + createGlobalOp(loc, name, type, /*isConstant=*/false); + } + return insertResult.first->second; + } + APValue *value = nullptr; if (mte->getStorageDuration() == SD_Static && varDecl->evaluateValue()) { // If the initializer of the extending declaration is a constant @@ -3762,8 +3771,7 @@ CIRGenModule::getAddrOfGlobalTemporary(const MaterializeTemporaryExpr *mte, linkage = cir::GlobalLinkageKind::InternalLinkage; } } - mlir::Location loc = getLoc(mte->getSourceRange()); - cir::GlobalOp gv = createGlobalOp(*this, loc, name, type, isConstant); + cir::GlobalOp gv = createGlobalOp(loc, name, type, isConstant); gv.setInitialValueAttr(initialValue); if (emitter) @@ -3786,7 +3794,10 @@ CIRGenModule::getAddrOfGlobalTemporary(const MaterializeTemporaryExpr *mte, assert(!cir::MissingFeatures::addressSpace()); // Update the map with the new temporary. If we created a placeholder above, - // replace it with the new global now. + // erase it as well, the name will have been the same, so our symbol + // references would have been correct. We still do a 'replaceAllUsesWith' in + // case some sort of expression formed a reference to the placeholder + // temporary. mlir::Operation *&entry = materializedGlobalTemporaryMap[mte]; if (entry) { entry->replaceAllUsesWith(cv); @@ -3829,7 +3840,7 @@ cir::GlobalOp CIRGenModule::getAddrOfUnnamedGlobalConstantDecl( std::string name = numEntries == 0 ? ".constant" : (Twine(".constant.") + Twine(numEntries)).str(); - auto globalOp = createGlobalOp(*this, builder.getUnknownLoc(), name, + auto globalOp = createGlobalOp(builder.getUnknownLoc(), name, typedInit.getType(), /*is_constant=*/true); globalOp.setLinkage(cir::GlobalLinkageKind::PrivateLinkage); @@ -3870,7 +3881,7 @@ CIRGenModule::getAddrOfTemplateParamObject(const TemplateParamObjectDecl *tpo) { : cir::GlobalLinkageKind::InternalLinkage; assert(!cir::MissingFeatures::addressSpace()); - auto globalOp = createGlobalOp(*this, builder.getUnknownLoc(), name, + auto globalOp = createGlobalOp(builder.getUnknownLoc(), name, typedInit.getType(), /*is_constant=*/true); globalOp.setLinkage(linkage); globalOp.setAlignment(alignment.getAsAlign().value()); diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h index e2184ef8640f3..dcd185da72b75 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -275,9 +275,9 @@ class CIRGenModule : public CIRGenTypeCache { cir::GlobalOp getOrCreateCIRGlobal(const VarDecl *d, mlir::Type ty, ForDefinition_t isForDefinition); - static cir::GlobalOp - createGlobalOp(CIRGenModule &cgm, mlir::Location loc, llvm::StringRef name, - mlir::Type t, bool isConstant = false, + cir::GlobalOp + createGlobalOp(mlir::Location loc, llvm::StringRef name, mlir::Type t, + bool isConstant = false, mlir::ptr::MemorySpaceAttrInterface addrSpace = {}, mlir::Operation *insertPoint = nullptr); diff --git a/clang/test/CIR/CodeGen/self-ref-temporaries.cpp b/clang/test/CIR/CodeGen/self-ref-temporaries.cpp new file mode 100644 index 0000000000000..a8fb4a8bcbbe8 --- /dev/null +++ b/clang/test/CIR/CodeGen/self-ref-temporaries.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - | FileCheck %s --check-prefix=CIR +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM +// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix=LLVM + + constexpr const int &normal = 42; +// CIR: cir.global "private" constant external @_ZGR6normal_ = #cir.int<42> : !s32i +// CIR: cir.global constant external @normal = #cir.global_view<@_ZGR6normal_> : !cir.ptr<!s32i> +// LLVM: @_ZGR6normal_ = {{.*}}constant i32 42, align 4 +// LLVM: @normal = constant ptr @_ZGR6normal_, align 8 + +struct SelfRef { + int *p = ints; + int ints[3] = {1, 2, 3}; +}; +constexpr const SelfRef &sr = SelfRef(); +// CIR: cir.global "private" constant external @_ZGR2sr_ = #cir.const_record<{#cir.global_view<@_ZGR2sr_, [1 : i32]> : !cir.ptr<!s32i>, #cir.const_array<[#cir.int<1> : !s32i, #cir.int<2> : !s32i, #cir.int<3> : !s32i]> : !cir.array<!s32i x 3>}> +// CIR: cir.global constant external @sr = #cir.global_view<@_ZGR2sr_> : !cir.ptr<!rec_SelfRef> +// LLVM: @_ZGR2sr_ = {{.*}}constant { ptr, [3 x i32] } { ptr getelementptr {{.*}}(i8, ptr @_ZGR2sr_, i64 8), [3 x i32] [i32 1, i32 2, i32 3] }, align 8 +// LLVM: @sr = constant ptr @_ZGR2sr_, align 8 + +struct MultiSelfRef { + int *p = ints; + int *q = ints; + int ints[3] = {1, 2, 3}; +}; + +constexpr const MultiSelfRef &msr = MultiSelfRef(); +// CIR: cir.global "private" constant external @_ZGR3msr_ = #cir.const_record<{#cir.global_view<@_ZGR3msr_, [2 : i32]> : !cir.ptr<!s32i>, #cir.global_view<@_ZGR3msr_, [2 : i32]> : !cir.ptr<!s32i>, #cir.const_array<[#cir.int<1> : !s32i, #cir.int<2> : !s32i, #cir.int<3> : !s32i]> : !cir.array<!s32i x 3>}> +// CIR: cir.global constant external @msr = #cir.global_view<@_ZGR3msr_> : !cir.ptr<!rec_MultiSelfRef> +// LLVM: @_ZGR3msr_ = {{.*}}constant { ptr, ptr, [3 x i32] } { ptr getelementptr {{.*}}(i8, ptr @_ZGR3msr_, i64 16), ptr getelementptr {{.*}}(i8, ptr @_ZGR3msr_, i64 16), [3 x i32] [i32 1, i32 2, i32 3] }, align 8 +// LLVM: @msr = constant ptr @_ZGR3msr_, align 8 + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
