https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/187327
>From 0e50d2c2851b49c48a3a38857102d122608b007a Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 11 Mar 2026 06:54:27 -0700 Subject: [PATCH 1/6] [CIR] Implement member-pointer members lowering/CXX ABI lowering Record types with a member pointer as a member require quite a bit of work to get to function properly. First, we have to wire them through the AST->CIR lowering to make sure we properly represent them, and represent them when they're zero initializable. We also have to properly initialize elements when we're NOT zero initializable. More importantly, we have to implement the CXXABILowering of record types. Before this patch, we just assumed that all RecordTypes were legal, since we didn't have the above lowering. A vast majority of this patch is around getting RecordTypes to lower properly. There isn't really a good way to test this without the FE changes, so it wasn't split off. We accomplish this in 2 phases: First, we transform each individual record type along the way, giving it a new cxx-abi specific name. We have to ensure that recursive evaluation works correctly, so we pulled the solution from the LLVM-IR dialect for that. Secondly, we rename all of the RecordType's back. This is safe only after we have 'removed' all references to the untransformed RecordType due to the way RecordStorage works, however it'll assert if we get this wrong. We do this second stage to minimize the impact on tests as well as the IR: otherwise EVERY RecordType would have their names changed. In addition to massive test changes, this would be an ugly change everywhere. One additional concern with RecordType lowering, is that it makes most of RecordTypes potentially 'illegal'. Thus, any attribute that 'references' a record type is required to be transformed as well, so this patch implements attribute legalization and transformation. This quite a few NYIs around lowering of these, though most are probably not really accessible. --- .../include/clang/CIR/Dialect/IR/CIRTypes.td | 29 +- clang/lib/CIR/CodeGen/CIRGenCXXABI.h | 4 + clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 23 +- clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp | 6 + clang/lib/CIR/CodeGen/CIRGenTypes.cpp | 12 +- clang/lib/CIR/CodeGen/CIRGenerator.cpp | 2 +- clang/lib/CIR/Dialect/IR/CIRTypes.cpp | 22 +- .../CIR/Dialect/Transforms/CXXABILowering.cpp | 522 ++++++++++++++++-- clang/test/CIR/CodeGen/nonzeroinit-struct.cpp | 84 ++- 9 files changed, 624 insertions(+), 80 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td index 450c02135e033..1ff55a2b21ee1 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRTypes.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRTypes.td @@ -315,9 +315,10 @@ def CIR_PointerType : CIR_Type<"Pointer", "ptr", [ // CIR_DataMemberType //===----------------------------------------------------------------------===// -def CIR_DataMemberType : CIR_Type<"DataMember", "data_member", - [DeclareTypeInterfaceMethods<DataLayoutTypeInterface>] -> { +def CIR_DataMemberType : CIR_Type<"DataMember", "data_member", [ + DeclareTypeInterfaceMethods<DataLayoutTypeInterface>, + DeclareTypeInterfaceMethods<CIR_SizedTypeInterface> +]> { let summary = "CIR type that represents a pointer-to-data-member in C++"; let description = [{ `cir.data_member` models a pointer-to-data-member in C++. Values of this @@ -561,8 +562,10 @@ def CIR_FuncType : CIR_Type<"Func", "func"> { // MethodType //===----------------------------------------------------------------------===// -def CIR_MethodType : CIR_Type<"Method", "method", - [DeclareTypeInterfaceMethods<DataLayoutTypeInterface>]> { +def CIR_MethodType : CIR_Type<"Method", "method", [ + DeclareTypeInterfaceMethods<DataLayoutTypeInterface>, + DeclareTypeInterfaceMethods<CIR_SizedTypeInterface> +]> { let summary = "CIR type that represents C++ pointer-to-member-function type"; let description = [{ `cir.method` models the pointer-to-member-function type in C++. The layout @@ -745,7 +748,23 @@ def CIR_RecordType : CIR_Type<"Record", "record", [ bool isLayoutIdentical(const RecordType &other); + // Checks the name of this record to check if it is a 'after' (or during) + // ABI converison. + bool isABIConvertedRecord() const; + + // Get the version of this record's name for use during cxx-abi conversion. + mlir::StringAttr getABIConvertedName() const; + + // This function should only be called after the CXXABILowering pass. It is + // intended to do a 'fixup' of the name so that we don't end up with messy + // looking CIR after CXXABI lowering. This should only be called if all uses + // of the pre-CXXABILowering version have been removed, otherwise we'll have + // a conflict in RecordStorage because two types will have the same hash. + void removeABIConversionNamePrefix(); + private: + + static constexpr char abi_conversion_prefix[] = "__post_abi_"; unsigned computeStructSize(const mlir::DataLayout &dataLayout) const; uint64_t computeStructAlignment(const mlir::DataLayout &dataLayout) const; public: diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h index 1e5bd990b0737..614d62fda44a5 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h @@ -381,6 +381,10 @@ class CIRGenCXXABI { const CXXNewExpr *e, QualType elementType) = 0; + /// Return true if the given member pointer can be zero-initialized + /// (in the C++ sense). + virtual bool isZeroInitializable(const MemberPointerType *mpt) = 0; + protected: /// Returns the extra size required in order to store the array /// cookie for the given type. Assumes that an array cookie is diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp index c5c5847da1d48..9b5cf25983ee0 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp @@ -1603,10 +1603,9 @@ static mlir::TypedAttr emitNullConstant(CIRGenModule &cgm, const RecordDecl *rd, // Now go through all other fields and zero them out. for (unsigned i = 0; i != numElements; ++i) { - if (!elements[i]) { - cgm.errorNYI(rd->getSourceRange(), "emitNullConstant: field not zeroed"); - return {}; - } + if (!elements[i]) + elements[i] = + cgm.getBuilder().getZeroInitAttr(recordTy.getElementType(i)); } mlir::MLIRContext *mlirContext = recordTy.getContext(); @@ -1643,15 +1642,10 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &d) { // assignments and whatnots). Since this is for globals shouldn't // be a problem for the near future. if (cd->isTrivial() && cd->isDefaultConstructor()) { - const auto *cxxrd = ty->castAsCXXRecordDecl(); - if (!cgm.getTypes().isZeroInitializable(cxxrd)) { - // To handle this case, we really need to go through - // emitNullConstant, but we need an attribute, not a value - cgm.errorNYI( - "tryEmitPrivateForVarInit: non-zero-initializable cxx record"); - return {}; - } - return cir::ZeroAttr::get(cgm.convertType(d.getType())); + return cgm.emitNullConstantAttr(d.getType()); + // TODO: ERICH: main does the following line. Figure out which is + // right. + // return cir::ZeroAttr::get(cgm.convertType(d.getType())); } } } @@ -1976,8 +1970,7 @@ mlir::TypedAttr CIRGenModule::emitNullConstantAttr(QualType t) { assert(t->isMemberDataPointerType() && "Should only see pointers to data members here!"); - errorNYI("CIRGenModule::emitNullConstantAttr unsupported type"); - return {}; + return emitNullMemberAttr(t, t->castAs<MemberPointerType>()); } mlir::TypedAttr diff --git a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp index a704923e73685..6e2e5beab1cb4 100644 --- a/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp @@ -196,6 +196,8 @@ class CIRGenItaniumCXXABI : public CIRGenCXXABI { mlir::Value numElements, const CXXNewExpr *e, QualType elementType) override; + bool isZeroInitializable(const MemberPointerType *MPT) override; + protected: CharUnits getArrayCookieSizeImpl(QualType elementType) override; @@ -2795,3 +2797,7 @@ mlir::Value CIRGenItaniumCXXABI::performReturnAdjustment( ra.Virtual.Itanium.VBaseOffsetOffset, /*isReturnAdjustment=*/true); } + +bool CIRGenItaniumCXXABI::isZeroInitializable(const MemberPointerType *mpt) { + return mpt->isMemberFunctionPointer(); +} diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp index 2092964bac065..fc7e061f555d7 100644 --- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp @@ -1,5 +1,6 @@ #include "CIRGenTypes.h" +#include "CIRGenCXXABI.h" #include "CIRGenFunctionInfo.h" #include "CIRGenModule.h" #include "mlir/IR/BuiltinTypes.h" @@ -652,11 +653,12 @@ bool CIRGenTypes::isZeroInitializable(clang::QualType t) { if (const auto *rd = t->getAsRecordDecl()) return isZeroInitializable(rd); - if (t->getAs<MemberPointerType>()) { - cgm.errorNYI(SourceLocation(), "isZeroInitializable for MemberPointerType", - t); - return false; - } + if (const auto *mpt = t->getAs<MemberPointerType>()) + return theCXXABI.isZeroInitializable(mpt); + + if (t->getAs<HLSLInlineSpirvType>()) + cgm.errorNYI(SourceLocation(), + "isZeroInitializable for HLSLInlineSpirvType"); return true; } diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp index 6453f3565c33d..80f85169b73cb 100644 --- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp @@ -12,7 +12,7 @@ #include "CIRGenModule.h" -#include "mlir/Dialect/OpenACC/OpenACC.h" +#include "mlir/Dialect/OpenACC/OpenACCOpsDialect.h.inc" #include "mlir/Dialect/OpenMP/OpenMPDialect.h" #include "mlir/IR/MLIRContext.h" #include "mlir/Target/LLVMIR/Import.h" diff --git a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp index 80dce3d3266b5..0fa2e56ee26ec 100644 --- a/clang/lib/CIR/Dialect/IR/CIRTypes.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRTypes.cpp @@ -295,6 +295,25 @@ bool RecordType::getPadded() const { return getImpl()->padded; } cir::RecordType::RecordKind RecordType::getKind() const { return getImpl()->kind; } +bool RecordType::isABIConvertedRecord() const { + return getName() && getName().getValue().starts_with(abi_conversion_prefix); +} + +mlir::StringAttr RecordType::getABIConvertedName() const { + assert(!isABIConvertedRecord()); + + return StringAttr::get(getContext(), + abi_conversion_prefix + getName().getValue()); +} + +void RecordType::removeABIConversionNamePrefix() { + mlir::StringAttr recordName = getName(); + + if (recordName && recordName.getValue().starts_with(abi_conversion_prefix)) + getImpl()->name = mlir::StringAttr::get( + recordName.getValue().drop_front(sizeof(abi_conversion_prefix) - 1), + recordName.getType()); +} void RecordType::complete(ArrayRef<Type> members, bool packed, bool padded) { assert(!cir::MissingFeatures::astRecordDeclAttr()); @@ -774,7 +793,8 @@ MethodType::getTypeSizeInBits(const mlir::DataLayout &dataLayout, uint64_t MethodType::getABIAlignment(const mlir::DataLayout &dataLayout, mlir::DataLayoutEntryListRef params) const { - return dataLayout.getTypeSizeInBits(getMethodLayoutType(getContext())); + return cast<cir::RecordType>(getMethodLayoutType(getContext())) + .getABIAlignment(dataLayout, params); } //===----------------------------------------------------------------------===// diff --git a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp index c182c3a4310a2..d6df2748338ef 100644 --- a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp @@ -11,6 +11,7 @@ #include "PassDetail.h" #include "TargetLowering/LowerModule.h" +#include "mlir/Dialect/OpenACC/OpenACCOpsDialect.h.inc" #include "mlir/IR/PatternMatch.h" #include "mlir/Interfaces/DataLayoutInterfaces.h" #include "mlir/Pass/Pass.h" @@ -23,6 +24,9 @@ #include "clang/CIR/Dialect/Passes.h" #include "clang/CIR/MissingFeatures.h" +#include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/TypeSwitch.h" + using namespace mlir; using namespace cir; @@ -32,6 +36,208 @@ namespace mlir { } // namespace mlir namespace { +// Check an attribute for legality. An attribute is only currently potentially +// illegal if it contains a type, member pointers are our source of illegality +// in regards to attributes. +bool isCXXABIAttributeLegal(const mlir::TypeConverter &tc, + mlir::Attribute attr) { + // If we don't have an attribute, it can't have a type! + if (!attr) + return true; + + // None of the OpenACC attributes contain a type of concern, so we can just + // treat them as legal. + if (isa<mlir::acc::OpenACCDialect>(attr.getDialect())) + return true; + + // These attributes either don't contain a type, or don't contain a type that + // can have a data member/method. + if (isa<cir::IntAttr, cir::BoolAttr, cir::SourceLanguageAttr, + cir::OptInfoAttr, cir::ConstVectorAttr, cir::FPAttr, + cir::CUDAKernelNameAttr, cir::VisibilityAttr, cir::GlobalCtorAttr, + cir::GlobalDtorAttr, cir::LangAddressSpaceAttr, + cir::TargetAddressSpaceAttr, cir::BitfieldInfoAttr, + cir::AddressPointAttr, cir::BlockAddrInfoAttr, + cir::StaticLocalGuardAttr, cir::UsualDeleteParamsAttr, + cir::ASTVarDeclAttr, cir::CUDAExternallyInitializedAttr, + cir::CleanupKindAttr, cir::CleanupKindAttr, cir::ConstComplexAttr, + cir::CatchAllAttr, cir::UnwindAttr, cir::BlockAddrInfoAttr, + mlir::DenseArrayAttr, mlir::FloatAttr, mlir::UnitAttr, + mlir::StringAttr, mlir::IntegerAttr, mlir::SymbolRefAttr>(attr)) + return true; + + // Data Member and method are ALWAYS illegal. + if (isa<cir::DataMemberAttr, cir::MethodAttr>(attr)) + return false; + + return llvm::TypeSwitch<mlir::Attribute, bool>(attr) + // These attributes just have a type, so they are legal if their type is. + .Case<cir::ZeroAttr>( + [tc](cir::ZeroAttr za) { return tc.isLegal(za.getType()); }) + .Case<cir::PoisonAttr>( + [tc](cir::PoisonAttr pa) { return tc.isLegal(pa.getType()); }) + .Case<cir::UndefAttr>( + [tc](cir::UndefAttr uda) { return tc.isLegal(uda.getType()); }) + .Case<mlir::TypeAttr>( + [tc](mlir::TypeAttr ta) { return tc.isLegal(ta.getValue()); }) + .Case<cir::ConstPtrAttr>( + [tc](cir::ConstPtrAttr cpa) { return tc.isLegal(cpa.getType()); }) + .Case<cir::CXXCtorAttr>( + [tc](cir::CXXCtorAttr ca) { return tc.isLegal(ca.getType()); }) + .Case<cir::CXXDtorAttr>( + [tc](cir::CXXDtorAttr da) { return tc.isLegal(da.getType()); }) + .Case<cir::CXXAssignAttr>( + [tc](cir::CXXAssignAttr aa) { return tc.isLegal(aa.getType()); }) + + // Collection attributes are legal if ALL of the attributes in them are + // also legal. + .Case<mlir::ArrayAttr>([tc](mlir::ArrayAttr array) { + return std::all_of(array.getValue().begin(), array.getValue().end(), + [tc](mlir::Attribute attr) { + return isCXXABIAttributeLegal(tc, attr); + }); + }) + .Case<mlir::DictionaryAttr>([tc](mlir::DictionaryAttr dict) { + return std::all_of(dict.getValue().begin(), dict.getValue().end(), + [tc](mlir::NamedAttribute na) { + return isCXXABIAttributeLegal(tc, na.getValue()); + }); + }) + // These attributes have sub-attributes that we should check for legality. + .Case<cir::ConstArrayAttr>([tc](cir::ConstArrayAttr array) { + return tc.isLegal(array.getType()) && + isCXXABIAttributeLegal(tc, array.getElts()); + }) + .Case<cir::GlobalViewAttr>([tc](cir::GlobalViewAttr gva) { + return tc.isLegal(gva.getType()) && + isCXXABIAttributeLegal(tc, gva.getIndices()); + }) + .Case<cir::VTableAttr>([tc](cir::VTableAttr vta) { + return tc.isLegal(vta.getType()) && + isCXXABIAttributeLegal(tc, vta.getData()); + }) + .Case<cir::TypeInfoAttr>([tc](cir::TypeInfoAttr tia) { + return tc.isLegal(tia.getType()) && + isCXXABIAttributeLegal(tc, tia.getData()); + }) + .Case<cir::DynamicCastInfoAttr>([tc](cir::DynamicCastInfoAttr dcia) { + return isCXXABIAttributeLegal(tc, dcia.getSrcRtti()) && + isCXXABIAttributeLegal(tc, dcia.getDestRtti()) && + isCXXABIAttributeLegal(tc, dcia.getRuntimeFunc()) && + isCXXABIAttributeLegal(tc, dcia.getBadCastFunc()); + }) + .Case<cir::ConstRecordAttr>([tc](cir::ConstRecordAttr cra) { + return tc.isLegal(cra.getType()) && + isCXXABIAttributeLegal(tc, cra.getMembers()); + }) + // We did an audit of all of our attributes (both in OpenACC and CIR), so + // it shouldn't be dangerous to consider everything we haven't considered + // 'illegal'. Any 'new' attributes will end up asserting in + // 'rewriteAttribute' to make sure we consider them here. Otherwise, we + // wouldn't discover a problematic new attribute until it contains a + // member/method. + .Default(false); +} + +mlir::Attribute rewriteAttribute(const mlir::TypeConverter &tc, + mlir::MLIRContext *ctx, mlir::Attribute attr) { + // If the attribute is legal, there is no reason to rewrite it. This also + // filters out 'null' attributes. + if (isCXXABIAttributeLegal(tc, attr)) + return attr; + + // This switch needs to be kept in sync with the potentially-legal type switch + // from isCXXABIAttributeLegal. IF we miss any, this will end up causing + // verification/transformation issues later, often in the form of + // unrealized-conversion-casts. + + return llvm::TypeSwitch<mlir::Attribute, mlir::Attribute>(attr) + // These attributes just have a type, so convert just the type. + .Case<cir::ZeroAttr>([tc](cir::ZeroAttr za) { + return cir::ZeroAttr::get(tc.convertType(za.getType())); + }) + .Case<cir::PoisonAttr>([tc](cir::PoisonAttr pa) { + return cir::PoisonAttr::get(tc.convertType(pa.getType())); + }) + .Case<cir::UndefAttr>([tc](cir::UndefAttr uda) { + return cir::UndefAttr::get(tc.convertType(uda.getType())); + }) + .Case<mlir::TypeAttr>([tc](mlir::TypeAttr ta) { + return mlir::TypeAttr::get(tc.convertType(ta.getValue())); + }) + .Case<cir::ConstPtrAttr>([tc](cir::ConstPtrAttr cpa) { + return cir::ConstPtrAttr::get(tc.convertType(cpa.getType()), + cpa.getValue()); + }) + .Case<cir::CXXCtorAttr>([tc](cir::CXXCtorAttr ca) { + return cir::CXXCtorAttr::get(tc.convertType(ca.getType()), + ca.getCtorKind(), ca.getIsTrivial()); + }) + .Case<cir::CXXDtorAttr>([tc](cir::CXXDtorAttr da) { + return cir::CXXDtorAttr::get(tc.convertType(da.getType()), + da.getIsTrivial()); + }) + .Case<cir::CXXAssignAttr>([tc](cir::CXXAssignAttr aa) { + return cir::CXXAssignAttr::get(tc.convertType(aa.getType()), + aa.getAssignKind(), aa.getIsTrivial()); + }) + // Collection attributes need to transform all of the attributes inside of + // them. + .Case<mlir::ArrayAttr>([tc, ctx](mlir::ArrayAttr array) { + llvm::SmallVector<mlir::Attribute> elts; + for (mlir::Attribute a : array.getValue()) + elts.push_back(rewriteAttribute(tc, ctx, a)); + return mlir::ArrayAttr::get(ctx, elts); + }) + .Case<mlir::DictionaryAttr>([tc, ctx](mlir::DictionaryAttr dict) { + llvm::SmallVector<mlir::NamedAttribute> elts; + for (mlir::NamedAttribute na : dict.getValue()) + elts.emplace_back(na.getName(), + rewriteAttribute(tc, ctx, na.getValue())); + + return mlir::DictionaryAttr::get(ctx, elts); + }) + // These attributes have sub-attributes that need converting too. + .Case<cir::ConstArrayAttr>([tc, ctx](cir::ConstArrayAttr array) { + return cir::ConstArrayAttr::get( + ctx, tc.convertType(array.getType()), + rewriteAttribute(tc, ctx, array.getElts()), + array.getTrailingZerosNum()); + }) + .Case<cir::GlobalViewAttr>([tc, ctx](cir::GlobalViewAttr gva) { + return cir::GlobalViewAttr::get( + tc.convertType(gva.getType()), gva.getSymbol(), + mlir::cast<mlir::ArrayAttr>( + rewriteAttribute(tc, ctx, gva.getIndices()))); + }) + .Case<cir::VTableAttr>([tc, ctx](cir::VTableAttr vta) { + return cir::VTableAttr::get( + tc.convertType(vta.getType()), + mlir::cast<mlir::ArrayAttr>( + rewriteAttribute(tc, ctx, vta.getData()))); + }) + .Case<cir::TypeInfoAttr>([tc, ctx](cir::TypeInfoAttr tia) { + return cir::TypeInfoAttr::get( + tc.convertType(tia.getType()), + mlir::cast<mlir::ArrayAttr>( + rewriteAttribute(tc, ctx, tia.getData()))); + }) + .Case<cir::DynamicCastInfoAttr>([tc, ctx](cir::DynamicCastInfoAttr dcia) { + return cir::DynamicCastInfoAttr::get( + mlir::cast<cir::GlobalViewAttr>( + rewriteAttribute(tc, ctx, dcia.getSrcRtti())), + mlir::cast<cir::GlobalViewAttr>( + rewriteAttribute(tc, ctx, dcia.getDestRtti())), + dcia.getRuntimeFunc(), dcia.getBadCastFunc(), dcia.getOffsetHint()); + }) + .Case<cir::ConstRecordAttr>([tc, ctx](cir::ConstRecordAttr cra) { + return cir::ConstRecordAttr::get( + ctx, tc.convertType(cra.getType()), + mlir::cast<mlir::ArrayAttr>( + rewriteAttribute(tc, ctx, cra.getMembers()))); + }) + .DefaultUnreachable("unrewritten illegal attribute kind"); +} #define GET_ABI_LOWERING_PATTERNS #include "clang/CIR/Dialect/IR/CIRLowering.inc" @@ -80,14 +286,18 @@ class CIRGenericCXXABILoweringPattern : public mlir::ConversionPattern { return mlir::failure(); } - assert(op->getNumRegions() == 0 && "CIRGenericCXXABILoweringPattern cannot " - "deal with operations with regions"); - mlir::OperationState loweredOpState(op->getLoc(), op->getName()); loweredOpState.addOperands(operands); - loweredOpState.addAttributes(op->getAttrs()); loweredOpState.addSuccessors(op->getSuccessors()); + // Lower all attributes. + llvm::SmallVector<mlir::NamedAttribute> attrs; + for (const mlir::NamedAttribute &na : op->getAttrs()) + attrs.push_back( + {na.getName(), + rewriteAttribute(*typeConverter, op->getContext(), na.getValue())}); + loweredOpState.addAttributes(attrs); + // Lower all result types llvm::SmallVector<mlir::Type> loweredResultTypes; loweredResultTypes.reserve(op->getNumResults()); @@ -137,39 +347,44 @@ mlir::LogicalResult CIRCastOpABILowering::matchAndRewrite( cir::CastOp op, OpAdaptor adaptor, mlir::ConversionPatternRewriter &rewriter) const { mlir::Type srcTy = op.getSrc().getType(); - assert((mlir::isa<cir::DataMemberType, cir::MethodType>(srcTy)) && - "input to bitcast in ABI lowering must be a data member or method"); - - switch (op.getKind()) { - case cir::CastKind::bitcast: { - mlir::Type destTy = getTypeConverter()->convertType(op.getType()); - mlir::Value loweredResult; - if (mlir::isa<cir::DataMemberType>(srcTy)) - loweredResult = lowerModule->getCXXABI().lowerDataMemberBitcast( - op, destTy, adaptor.getSrc(), rewriter); - else - loweredResult = lowerModule->getCXXABI().lowerMethodBitcast( - op, destTy, adaptor.getSrc(), rewriter); - rewriter.replaceOp(op, loweredResult); - return mlir::success(); - } - case cir::CastKind::member_ptr_to_bool: { - mlir::Value loweredResult; - if (mlir::isa<cir::MethodType>(srcTy)) - loweredResult = lowerModule->getCXXABI().lowerMethodToBoolCast( - op, adaptor.getSrc(), rewriter); - else - loweredResult = lowerModule->getCXXABI().lowerDataMemberToBoolCast( - op, adaptor.getSrc(), rewriter); - rewriter.replaceOp(op, loweredResult); - return mlir::success(); - } - default: - break; + + if (mlir::isa<cir::DataMemberType, cir::MethodType>(srcTy)) { + switch (op.getKind()) { + case cir::CastKind::bitcast: { + mlir::Type destTy = getTypeConverter()->convertType(op.getType()); + mlir::Value loweredResult; + if (mlir::isa<cir::DataMemberType>(srcTy)) + loweredResult = lowerModule->getCXXABI().lowerDataMemberBitcast( + op, destTy, adaptor.getSrc(), rewriter); + else if (mlir::isa<cir::MethodType>(srcTy)) + loweredResult = lowerModule->getCXXABI().lowerMethodBitcast( + op, destTy, adaptor.getSrc(), rewriter); + rewriter.replaceOp(op, loweredResult); + return mlir::success(); + } + case cir::CastKind::member_ptr_to_bool: { + mlir::Value loweredResult; + if (mlir::isa<cir::DataMemberType>(srcTy)) + loweredResult = lowerModule->getCXXABI().lowerDataMemberToBoolCast( + op, adaptor.getSrc(), rewriter); + else if (mlir::isa<cir::MethodType>(srcTy)) + loweredResult = lowerModule->getCXXABI().lowerMethodToBoolCast( + op, adaptor.getSrc(), rewriter); + rewriter.replaceOp(op, loweredResult); + return mlir::success(); + } + default: + break; + } } - return mlir::failure(); + mlir::Value loweredResult = cir::CastOp::create( + rewriter, op.getLoc(), getTypeConverter()->convertType(op.getType()), + adaptor.getKind(), adaptor.getSrc()); + rewriter.replaceOp(op, loweredResult); + return mlir::success(); } + // Helper function to lower a value for things like an initializer. static mlir::TypedAttr lowerInitialValue(const LowerModule *lowerModule, const mlir::DataLayout &layout, @@ -188,11 +403,13 @@ static mlir::TypedAttr lowerInitialValue(const LowerModule *lowerModule, if (auto arrTy = mlir::dyn_cast<cir::ArrayType>(ty)) { auto loweredArrTy = mlir::cast<cir::ArrayType>(tc.convertType(arrTy)); - // TODO(cir): there are other types that can appear here inside of record - // members that we should handle. Those will come in a follow-up patch to - // minimize changes here. + if (!initVal) return {}; + + if (auto zeroVal = mlir::dyn_cast_if_present<cir::ZeroAttr>(initVal)) + return cir::ZeroAttr::get(loweredArrTy); + auto arrayVal = mlir::cast<cir::ConstArrayAttr>(initVal); auto arrayElts = mlir::cast<ArrayAttr>(arrayVal.getElts()); SmallVector<mlir::Attribute> loweredElements; @@ -208,8 +425,56 @@ static mlir::TypedAttr lowerInitialValue(const LowerModule *lowerModule, arrayVal.getTrailingZerosNum()); } - llvm_unreachable("inputs to cir.global/constant in ABI lowering must be data " - "member or method"); + if (auto recordTy = mlir::dyn_cast<cir::RecordType>(ty)) { + auto convertedTy = mlir::cast<cir::RecordType>(tc.convertType(recordTy)); + + if (auto recVal = mlir::dyn_cast_if_present<cir::ZeroAttr>(initVal)) + return cir::ZeroAttr::get(convertedTy); + + if (auto undefVal = mlir::dyn_cast_if_present<cir::UndefAttr>(initVal)) + return cir::UndefAttr::get(convertedTy); + + if (auto recVal = + mlir::dyn_cast_if_present<cir::ConstRecordAttr>(initVal)) { + auto recordMembers = mlir::cast<ArrayAttr>(recVal.getMembers()); + + SmallVector<mlir::Attribute> loweredMembers; + loweredMembers.reserve(recordMembers.size()); + + for (const mlir::Attribute &attr : recordMembers) { + auto typedAttr = cast<mlir::TypedAttr>(attr); + loweredMembers.push_back(lowerInitialValue( + lowerModule, layout, tc, typedAttr.getType(), typedAttr)); + } + + return cir::ConstRecordAttr::get( + convertedTy, mlir::ArrayAttr::get(ty.getContext(), loweredMembers)); + } + + assert(!initVal && "Record init val type not handled"); + return {}; + } + + // Pointers can contain record types, which can change. + if (auto ptrTy = mlir::dyn_cast<cir::PointerType>(ty)) { + auto convertedTy = mlir::cast<cir::PointerType>(tc.convertType(ptrTy)); + // pointers don't change other than their types. + + if (auto gva = mlir::dyn_cast_if_present<cir::GlobalViewAttr>(initVal)) + return cir::GlobalViewAttr::get(convertedTy, gva.getSymbol(), + gva.getIndices()); + + auto constPtr = mlir::cast_if_present<cir::ConstPtrAttr>(initVal); + if (!constPtr) + return {}; + return cir::ConstPtrAttr::get(convertedTy, constPtr.getValue()); + } + + assert(ty == tc.convertType(ty) && + "cir.global or constant operand is not an CXXABI-dependent type"); + + // Every other type can be left alone. + return cast<mlir::TypedAttr>(initVal); } mlir::LogicalResult CIRConstantOpABILowering::matchAndRewrite( @@ -227,16 +492,18 @@ mlir::LogicalResult CIRCmpOpABILowering::matchAndRewrite( cir::CmpOp op, OpAdaptor adaptor, mlir::ConversionPatternRewriter &rewriter) const { mlir::Type type = op.getLhs().getType(); - assert((mlir::isa<cir::DataMemberType, cir::MethodType>(type)) && - "input to cmp in ABI lowering must be a data member or method"); mlir::Value loweredResult; if (mlir::isa<cir::DataMemberType>(type)) loweredResult = lowerModule->getCXXABI().lowerDataMemberCmp( op, adaptor.getLhs(), adaptor.getRhs(), rewriter); - else + else if (mlir::isa<cir::MethodType>(type)) loweredResult = lowerModule->getCXXABI().lowerMethodCmp( op, adaptor.getLhs(), adaptor.getRhs(), rewriter); + else + loweredResult = cir::CmpOp::create( + rewriter, op.getLoc(), getTypeConverter()->convertType(op.getType()), + adaptor.getKind(), adaptor.getLhs(), adaptor.getRhs()); rewriter.replaceOp(op, loweredResult); return mlir::success(); @@ -268,6 +535,15 @@ mlir::LogicalResult CIRFuncOpABILowering::matchAndRewrite( // Create a new cir.func operation for the CXXABI-lowered function. cir::FuncOp loweredFuncOp = rewriter.cloneWithoutRegions(op); loweredFuncOp.setFunctionType(loweredFuncType); + + llvm::SmallVector<mlir::NamedAttribute> attrs; + for (const mlir::NamedAttribute &na : op->getAttrs()) + attrs.push_back( + {na.getName(), rewriteAttribute(*getTypeConverter(), op->getContext(), + na.getValue())}); + + loweredFuncOp->setAttrs(attrs); + rewriter.inlineRegionBefore(op.getBody(), loweredFuncOp.getBody(), loweredFuncOp.end()); if (mlir::failed(rewriter.convertRegionTypes( @@ -446,11 +722,116 @@ mlir::LogicalResult CIRVTableGetTypeInfoOpABILowering::matchAndRewrite( return mlir::success(); } -// A type to handle type conversion for the CXXABILowering pass. +namespace { +// A small type to handle type conversion for the the CXXABILoweringPass. +// Even though this is a CIR-to-CIR pass, we are eliminating some CIR types. +// Most importantly, this pass solves recursive type conversion problems by +// keeping a call stack. class CIRABITypeConverter : public mlir::TypeConverter { + + mlir::MLIRContext &context; + + // Recursive structure detection. + // We store one entry per thread here, and rely on locking. This works the + // same way as the LLVM-IR lowering does it, which has a similar problem. + DenseMap<uint64_t, std::unique_ptr<SmallVector<cir::RecordType>>> + conversionCallStack; + llvm::sys::SmartRWMutex<true> callStackMutex; + + // In order to let us 'change the names' back after the fact, we collect them + // along the way. They should only be added/accessed via the thread-safe + // functions below. + llvm::SmallVector<cir::RecordType> convertedRecordTypes; + llvm::sys::SmartRWMutex<true> recordTypeMutex; + + // This provides a stack for the RecordTypes being processed on the current + // thread, which lets us solve recursive conversions. This implementation is + // cribbed from the LLVMTypeConverter which solves a similar but not identical + // problem. + SmallVector<cir::RecordType> &getCurrentThreadRecursiveStack() { + { + // Most of the time, the entry already exists in the map. + std::shared_lock<decltype(callStackMutex)> lock(callStackMutex, + std::defer_lock); + if (context.isMultithreadingEnabled()) + lock.lock(); + auto recursiveStack = conversionCallStack.find(llvm::get_threadid()); + if (recursiveStack != conversionCallStack.end()) + return *recursiveStack->second; + } + + // First time this thread gets here, we have to get an exclusive access to + // insert in the map + std::unique_lock<decltype(callStackMutex)> lock(callStackMutex); + auto recursiveStackInserted = conversionCallStack.insert( + std::make_pair(llvm::get_threadid(), + std::make_unique<SmallVector<cir::RecordType>>())); + return *recursiveStackInserted.first->second; + } + + void addConvertedRecordType(cir::RecordType rt) { + std::unique_lock<decltype(recordTypeMutex)> lock(recordTypeMutex); + convertedRecordTypes.push_back(rt); + } + + llvm::SmallVector<mlir::Type> convertRecordMemberTypes(cir::RecordType type) { + llvm::SmallVector<mlir::Type> loweredMemberTypes; + loweredMemberTypes.reserve(type.getNumElements()); + + if (mlir::failed(convertTypes(type.getMembers(), loweredMemberTypes))) + return {}; + + return loweredMemberTypes; + } + + cir::RecordType convertRecordType(cir::RecordType type) { + // Unnamed record types can't be referred to recursively, so we can just + // convert this one. It also doesn't have uniqueness problems, so we can + // just do a conversion on it. + if (!type.getName()) + return cir::RecordType::get( + type.getContext(), convertRecordMemberTypes(type), type.getPacked(), + type.getPadded(), type.getKind()); + + // If the type has already been converted, we can just return, since there + // is nothing to do. Also, if is incomplete, it can't have invalid members! + // So we can skip transforming it. + if (type.isIncomplete() || type.isABIConvertedRecord()) + return type; + + SmallVectorImpl<cir::RecordType> &recursiveStack = + getCurrentThreadRecursiveStack(); + + auto convertedType = cir::RecordType::get( + type.getContext(), type.getABIConvertedName(), type.getKind()); + + // This type has already been converted, just return it. + if (convertedType.isComplete()) + return convertedType; + + // We put the existing 'type' into the vector if we're in the process of + // converting it (and pop it when we're done). To prevent recursion, + // just return the 'incomplete' version, and the 'top level' version of this + // call will call 'complete' on it. + if (llvm::is_contained(recursiveStack, type)) + return convertedType; + + recursiveStack.push_back(type); + llvm::scope_exit popConvertingType( + [&recursiveStack]() { recursiveStack.pop_back(); }); + + SmallVector<mlir::Type> convertedMembers = convertRecordMemberTypes(type); + + convertedType.complete(convertedMembers, type.getPacked(), + type.getPadded()); + addConvertedRecordType(convertedType); + return convertedType; + } + public: - CIRABITypeConverter(mlir::DataLayout &dataLayout, - cir::LowerModule &lowerModule) { + CIRABITypeConverter(mlir::MLIRContext &ctx, mlir::DataLayout &dataLayout, + cir::LowerModule &lowerModule) + : context(ctx) { addConversion([&](mlir::Type type) -> mlir::Type { return type; }); // This is necessary in order to convert CIR pointer types that are // pointing to CIR types that we are lowering in this pass. @@ -493,8 +874,19 @@ class CIRABITypeConverter : public mlir::TypeConverter { return cir::FuncType::get(loweredInputTypes, loweredReturnType, /*isVarArg=*/type.getVarArg()); }); + addConversion([&](cir::RecordType type) -> mlir::Type { + return convertRecordType(type); + }); + } + + void restoreRecordTypeNames() { + std::unique_lock<decltype(recordTypeMutex)> lock(recordTypeMutex); + + for (auto rt : convertedRecordTypes) + rt.removeABIConversionNamePrefix(); } }; +} // namespace static void populateCXXABIConversionTarget(mlir::ConversionTarget &target, @@ -508,7 +900,33 @@ populateCXXABIConversionTarget(mlir::ConversionTarget &target, [&typeConverter](mlir::Operation *op) { if (!typeConverter.isLegal(op)) return false; - return std::all_of(op->getRegions().begin(), op->getRegions().end(), + + bool attrs = std::all_of( + op->getAttrs().begin(), op->getAttrs().end(), + [&typeConverter](const mlir::NamedAttribute &a) { + return isCXXABIAttributeLegal(typeConverter, a.getValue()); + }); + + return attrs && + std::all_of(op->getRegions().begin(), op->getRegions().end(), + [&typeConverter](mlir::Region ®ion) { + return typeConverter.isLegal(®ion); + }); + }); + + target.addDynamicallyLegalDialect<mlir::acc::OpenACCDialect>( + [&typeConverter](mlir::Operation *op) { + if (!typeConverter.isLegal(op)) + return false; + + bool attrs = std::all_of( + op->getAttrs().begin(), op->getAttrs().end(), + [&typeConverter](const mlir::NamedAttribute &a) { + return isCXXABIAttributeLegal(typeConverter, a.getValue()); + }); + + return attrs && + std::all_of(op->getRegions().begin(), op->getRegions().end(), [&typeConverter](mlir::Region ®ion) { return typeConverter.isLegal(®ion); }); @@ -516,7 +934,13 @@ populateCXXABIConversionTarget(mlir::ConversionTarget &target, // Some CIR ops needs special checking for legality target.addDynamicallyLegalOp<cir::FuncOp>([&typeConverter](cir::FuncOp op) { - return typeConverter.isLegal(op.getFunctionType()); + bool attrs = std::all_of(op->getAttrs().begin(), op->getAttrs().end(), + [&typeConverter](const mlir::NamedAttribute &a) { + return isCXXABIAttributeLegal(typeConverter, + a.getValue()); + }); + + return attrs && typeConverter.isLegal(op.getFunctionType()); }); target.addDynamicallyLegalOp<cir::GlobalOp>( [&typeConverter](cir::GlobalOp op) { @@ -609,7 +1033,7 @@ void CXXABILoweringPass::runOnOperation() { } mlir::DataLayout dataLayout(mod); - CIRABITypeConverter typeConverter(dataLayout, *lowerModule); + CIRABITypeConverter typeConverter(*ctx, dataLayout, *lowerModule); mlir::RewritePatternSet patterns(ctx); patterns.add<CIRGenericCXXABILoweringPattern>(patterns.getContext(), @@ -629,6 +1053,8 @@ void CXXABILoweringPass::runOnOperation() { if (failed(mlir::applyPartialConversion(ops, target, std::move(patterns)))) signalPassFailure(); + + typeConverter.restoreRecordTypeNames(); } std::unique_ptr<Pass> mlir::createCXXABILoweringPass() { diff --git a/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp b/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp index 871a028901947..9e73d3d4f8798 100644 --- a/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp +++ b/clang/test/CIR/CodeGen/nonzeroinit-struct.cpp @@ -1,7 +1,18 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o - -verify +// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -fclangir -emit-cir -fcxx-exceptions -fexceptions -mmlir --mlir-print-ir-before=cir-cxxabi-lowering -o %t.cir 2> %t-before.cir +// RUN: FileCheck %s --input-file=%t-before.cir --check-prefixes=CIR,CIR-BEFORE +// RUN: FileCheck %s --input-file=%t.cir --check-prefixes=CIR,CIR-AFTER +// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -fclangir -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s --check-prefixes=LLVM +// RUN: %clang_cc1 %s -triple x86_64-apple-darwin10 -emit-llvm -fcxx-exceptions -fexceptions -o - | FileCheck %s --check-prefixes=LLVM struct Other { int x; + void func(int, float); +}; + +struct WithMemPtr { + int x; + double y; + void (Other::*mpt)(int, float); }; struct Trivial { @@ -10,11 +21,74 @@ struct Trivial { decltype(&Other::x) ptr; }; +// CIR-BEFORE-DAG: !rec_Other = !cir.record<struct "Other" {!s32i}> +// CIR-BEFORE-DAG: !rec_Trivial = !cir.record<struct "Trivial" {!s32i, !cir.double, !cir.data_member<!s32i in !rec_Other>}> +// CIR-BEFORE-DAG: !rec_WithMemPtr = !cir.record<struct "WithMemPtr" {!s32i, !cir.double, !cir.method<!cir.func<(!cir.ptr<!rec_Other>, !s32i, !cir.float)> in !rec_Other>}> +// CIR-AFTER-DAG: !rec_Other = !cir.record<struct "Other" {!s32i}> +// CIR-AFTER-DAG: !rec_Trivial = !cir.record<struct "Trivial" {!s32i, !cir.double, !s64i}> +// CIR-AFTER-DAG: !rec_anon_struct = !cir.record<struct {!s64i, !s64i}> +// CIR-AFTER-DAG: !rec_WithMemPtr = !cir.record<struct "WithMemPtr" {!s32i, !cir.double, !rec_anon_struct}> + +// LLVM-DAG: %struct.WithMemPtr = type { i32, double, { i64, i64 } } +// LLVM-DAG: %struct.Trivial = type { i32, double, i64 } + +// CIR-AFTER-DAG: cir.global "private" constant cir_private @__const.local.localT_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.200000e+00> : !cir.double, #cir.int<0> : !s64i}> : !rec_Trivial +// CIR-AFTER-DAG: cir.global "private" constant cir_private @__const.local.localMpt_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.000000e+00> : !cir.double, #cir.const_record<{#cir.global_view<@_ZN5Other4funcEif> : !s64i, #cir.int<0> : !s64i}> : !rec_anon_struct}> : !rec_WithMemPtr + +// LLVM-DAG: @__const.local.localMpt_init = private {{.*}}constant %struct.WithMemPtr { i32 1, double 2.000000e+00, { i64, i64 } { i64 ptrtoint (ptr @_ZN5Other4funcEif to i64), i64 0 } } +// LLVM-DAG: @__const.local.localT_init = private {{.*}}constant %struct.Trivial { i32 1, double 2.200000e+00, i64 0 } + +// This CAN be zero-initialized. +WithMemPtr mpt; +// CIR-DAG: cir.global external @mpt = #cir.zero : !rec_WithMemPtr {alignment = 8 : i64} +// LLVM-DAG: @mpt = global %struct.WithMemPtr zeroinitializer, align 8 + +WithMemPtr mpt_init{1, 2.0, &Other::func}; +// CIR-BEFORE-DAG: cir.global external @mpt_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.000000e+00> : !cir.double, #cir.method<@_ZN5Other4funcEif> : !cir.method<!cir.func<(!cir.ptr<!rec_Other>, !s32i, !cir.float)> in !rec_Other>}> : !rec_WithMemPtr {alignment = 8 : i64} +// CIR-AFTER-DAG: cir.global external @mpt_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.000000e+00> : !cir.double, #cir.const_record<{#cir.global_view<@_ZN5Other4funcEif> : !s64i, #cir.int<0> : !s64i}> : !rec_anon_struct}> : !rec_WithMemPtr {alignment = 8 : i64} +// LLVM-DAG: @mpt_init = global %struct.WithMemPtr { i32 1, double 2.000000e+00, { i64, i64 } { i64 ptrtoint (ptr @_ZN5Other4funcEif to i64), i64 0 } }, align 8 + // This case has a trivial default constructor, but can't be zero-initialized. Trivial t; +// CIR-BEFORE-DAG: cir.global external @t = #cir.const_record<{#cir.int<0> : !s32i, #cir.fp<0.000000e+00> : !cir.double, #cir.data_member<null> : !cir.data_member<!s32i in !rec_Other>}> : !rec_Trivial {alignment = 8 : i64} +// CIR-AFTER-DAG: cir.global external @t = #cir.const_record<{#cir.int<0> : !s32i, #cir.fp<0.000000e+00> : !cir.double, #cir.int<-1> : !s64i}> : !rec_Trivial {alignment = 8 : i64} loc(#loc25) +// LLVM-DAG: @t = global %struct.Trivial { i32 0, double 0.000000e+00, i64 -1 }, align 8 + +Trivial t_init{1,2.2, &Other::x}; +// CIR-BEFORE-DAG: cir.global external @t_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.200000e+00> : !cir.double, #cir.data_member<0> : !cir.data_member<!s32i in !rec_Other>}> : !rec_Trivial {alignment = 8 : i64} +// CIR-AFTER-DAG: cir.global external @t_init = #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.200000e+00> : !cir.double, #cir.int<0> : !s64i}> : !rec_Trivial {alignment = 8 : i64} +// LLVM-DAG: @t_init = global %struct.Trivial { i32 1, double 2.200000e+00, i64 0 }, align 8 + +extern "C" void local() { + // CIR-LABEL: @local( + // LLVM-LABEL: @local( + // CIR: cir.alloca !rec_WithMemPtr, !cir.ptr<!rec_WithMemPtr>, ["localMpt"] {alignment = 8 : i64} + // CIR: cir.alloca !rec_Trivial, !cir.ptr<!rec_Trivial>, ["localT"] {alignment = 8 : i64} + // CIR: %[[MPT_INIT:.*]] = cir.alloca !rec_WithMemPtr, !cir.ptr<!rec_WithMemPtr>, ["localMpt_init", init] {alignment = 8 : i64} + // CIR: %[[T_INIT:.*]] = cir.alloca !rec_Trivial, !cir.ptr<!rec_Trivial>, ["localT_init", init] {alignment = 8 : i64} + + // LLVM: alloca %struct.WithMemPtr + // LLVM: alloca %struct.Trivial + // LLVM: %[[MPT_INIT:.*]] = alloca %struct.WithMemPtr + // LLVM: %[[T_INIT:.*]] = alloca %struct.Trivial + WithMemPtr localMpt; + + Trivial localT; + + WithMemPtr localMpt_init{1, 2.0, &Other::func}; + // CIR-BEFORE: %[[MPT_INIT_VAL:.*]] = cir.const #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.000000e+00> : !cir.double, #cir.method<@_ZN5Other4funcEif> : !cir.method<!cir.func<(!cir.ptr<!rec_Other>, !s32i, !cir.float)> in !rec_Other>}> : !rec_WithMemPtr + // CIR-BEFORE: cir.store align(8) %[[MPT_INIT_VAL]], %[[MPT_INIT]] : !rec_WithMemPtr, !cir.ptr<!rec_WithMemPtr> + // CIR-AFTER: %[[MPT_INIT_VAL:.*]] = cir.get_global @__const.local.localMpt_init : !cir.ptr<!rec_WithMemPtr> + // CIR-AFTER: cir.copy %[[MPT_INIT_VAL]] to %[[MPT_INIT]] : !cir.ptr<!rec_WithMemPtr> + + // LLVM: call void @llvm.memcpy.p0.p0.i64(ptr {{.*}}%[[MPT_INIT]], ptr {{.*}}@__const.local.localMpt_init, i64 32, i1 false) + + Trivial localT_init{1,2.2, &Other::x}; + // CIR-BEFORE: %[[T_INIT_VAL:.*]] = cir.const #cir.const_record<{#cir.int<1> : !s32i, #cir.fp<2.200000e+00> : !cir.double, #cir.data_member<0> : !cir.data_member<!s32i in !rec_Other>}> : !rec_Trivial + // CIR-BEFORE: cir.store align(8) %[[T_INIT_VAL]], %[[T_INIT]] : !rec_Trivial, !cir.ptr<!rec_Trivial> -// Since the case above isn't handled yet, we want a test that verifies that -// we're failing for the right reason. + // CIR-AFTER: %[[T_INIT_VAL:.*]] = cir.get_global @__const.local.localT_init : !cir.ptr<!rec_Trivial> + // CIR-AFTER: cir.copy %[[T_INIT_VAL]] to %[[T_INIT]] : !cir.ptr<!rec_Trivial> -// expected-error@*:* {{ClangIR code gen Not Yet Implemented: isZeroInitializable for MemberPointerType}} -// expected-error@*:* {{ClangIR code gen Not Yet Implemented: tryEmitPrivateForVarInit: non-zero-initializable cxx record}} + // LLVM: call void @llvm.memcpy.p0.p0.i64(ptr {{.*}}%[[T_INIT]], ptr {{.*}}@__const.local.localT_init, i64 24, i1 false) +} >From 38d4d1f2c831e4f13c4219270253d67eec142677 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 18 Mar 2026 10:23:53 -0700 Subject: [PATCH 2/6] Remove leftover TODO from my refactor, I'm confident in the change I made after looking/analyzing again --- clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp index 9b5cf25983ee0..271d760244400 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp @@ -1641,12 +1641,8 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &d) { // just emitting a global with zero init (mimic what we do for trivial // assignments and whatnots). Since this is for globals shouldn't // be a problem for the near future. - if (cd->isTrivial() && cd->isDefaultConstructor()) { + if (cd->isTrivial() && cd->isDefaultConstructor()) return cgm.emitNullConstantAttr(d.getType()); - // TODO: ERICH: main does the following line. Figure out which is - // right. - // return cir::ZeroAttr::get(cgm.convertType(d.getType())); - } } } } >From f027b6b4099b8ca1782ef66fdf2f8371f02ce45e Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 18 Mar 2026 14:26:01 -0700 Subject: [PATCH 3/6] Do the 'simple' suggestions from Andy, more to come --- clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 4 - .../CIR/Dialect/Transforms/CXXABILowering.cpp | 108 +++++++++--------- 2 files changed, 53 insertions(+), 59 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp index 271d760244400..cb0227681955e 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp @@ -1637,10 +1637,6 @@ mlir::Attribute ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &d) { if (ty->isRecordType()) { if (const auto *e = dyn_cast_or_null<CXXConstructExpr>(d.getInit())) { const CXXConstructorDecl *cd = e->getConstructor(); - // FIXME: we should probably model this more closely to C++ than - // just emitting a global with zero init (mimic what we do for trivial - // assignments and whatnots). Since this is for globals shouldn't - // be a problem for the near future. if (cd->isTrivial() && cd->isDefaultConstructor()) return cgm.emitNullConstantAttr(d.getType()); } diff --git a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp index d6df2748338ef..20c69414937ad 100644 --- a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp @@ -73,60 +73,58 @@ bool isCXXABIAttributeLegal(const mlir::TypeConverter &tc, return llvm::TypeSwitch<mlir::Attribute, bool>(attr) // These attributes just have a type, so they are legal if their type is. .Case<cir::ZeroAttr>( - [tc](cir::ZeroAttr za) { return tc.isLegal(za.getType()); }) + [&tc](cir::ZeroAttr za) { return tc.isLegal(za.getType()); }) .Case<cir::PoisonAttr>( - [tc](cir::PoisonAttr pa) { return tc.isLegal(pa.getType()); }) + [&tc](cir::PoisonAttr pa) { return tc.isLegal(pa.getType()); }) .Case<cir::UndefAttr>( - [tc](cir::UndefAttr uda) { return tc.isLegal(uda.getType()); }) + [&tc](cir::UndefAttr uda) { return tc.isLegal(uda.getType()); }) .Case<mlir::TypeAttr>( - [tc](mlir::TypeAttr ta) { return tc.isLegal(ta.getValue()); }) + [&tc](mlir::TypeAttr ta) { return tc.isLegal(ta.getValue()); }) .Case<cir::ConstPtrAttr>( - [tc](cir::ConstPtrAttr cpa) { return tc.isLegal(cpa.getType()); }) + [&tc](cir::ConstPtrAttr cpa) { return tc.isLegal(cpa.getType()); }) .Case<cir::CXXCtorAttr>( - [tc](cir::CXXCtorAttr ca) { return tc.isLegal(ca.getType()); }) + [&tc](cir::CXXCtorAttr ca) { return tc.isLegal(ca.getType()); }) .Case<cir::CXXDtorAttr>( - [tc](cir::CXXDtorAttr da) { return tc.isLegal(da.getType()); }) + [&tc](cir::CXXDtorAttr da) { return tc.isLegal(da.getType()); }) .Case<cir::CXXAssignAttr>( - [tc](cir::CXXAssignAttr aa) { return tc.isLegal(aa.getType()); }) + [&tc](cir::CXXAssignAttr aa) { return tc.isLegal(aa.getType()); }) // Collection attributes are legal if ALL of the attributes in them are // also legal. - .Case<mlir::ArrayAttr>([tc](mlir::ArrayAttr array) { - return std::all_of(array.getValue().begin(), array.getValue().end(), - [tc](mlir::Attribute attr) { - return isCXXABIAttributeLegal(tc, attr); - }); + .Case<mlir::ArrayAttr>([&tc](mlir::ArrayAttr array) { + return llvm::all_of(array.getValue(), [&tc](mlir::Attribute attr) { + return isCXXABIAttributeLegal(tc, attr); + }); }) - .Case<mlir::DictionaryAttr>([tc](mlir::DictionaryAttr dict) { - return std::all_of(dict.getValue().begin(), dict.getValue().end(), - [tc](mlir::NamedAttribute na) { - return isCXXABIAttributeLegal(tc, na.getValue()); - }); + .Case<mlir::DictionaryAttr>([&tc](mlir::DictionaryAttr dict) { + return llvm::all_of(dict.getValue(), [&tc](mlir::NamedAttribute na) { + return isCXXABIAttributeLegal(tc, na.getValue()); + }); }) // These attributes have sub-attributes that we should check for legality. - .Case<cir::ConstArrayAttr>([tc](cir::ConstArrayAttr array) { + .Case<cir::ConstArrayAttr>([&tc](cir::ConstArrayAttr array) { return tc.isLegal(array.getType()) && isCXXABIAttributeLegal(tc, array.getElts()); }) - .Case<cir::GlobalViewAttr>([tc](cir::GlobalViewAttr gva) { + .Case<cir::GlobalViewAttr>([&tc](cir::GlobalViewAttr gva) { return tc.isLegal(gva.getType()) && isCXXABIAttributeLegal(tc, gva.getIndices()); }) - .Case<cir::VTableAttr>([tc](cir::VTableAttr vta) { + .Case<cir::VTableAttr>([&tc](cir::VTableAttr vta) { return tc.isLegal(vta.getType()) && isCXXABIAttributeLegal(tc, vta.getData()); }) - .Case<cir::TypeInfoAttr>([tc](cir::TypeInfoAttr tia) { + .Case<cir::TypeInfoAttr>([&tc](cir::TypeInfoAttr tia) { return tc.isLegal(tia.getType()) && isCXXABIAttributeLegal(tc, tia.getData()); }) - .Case<cir::DynamicCastInfoAttr>([tc](cir::DynamicCastInfoAttr dcia) { + .Case<cir::DynamicCastInfoAttr>([&tc](cir::DynamicCastInfoAttr dcia) { return isCXXABIAttributeLegal(tc, dcia.getSrcRtti()) && isCXXABIAttributeLegal(tc, dcia.getDestRtti()) && isCXXABIAttributeLegal(tc, dcia.getRuntimeFunc()) && isCXXABIAttributeLegal(tc, dcia.getBadCastFunc()); }) - .Case<cir::ConstRecordAttr>([tc](cir::ConstRecordAttr cra) { + .Case<cir::ConstRecordAttr>([&tc](cir::ConstRecordAttr cra) { return tc.isLegal(cra.getType()) && isCXXABIAttributeLegal(tc, cra.getMembers()); }) @@ -153,43 +151,43 @@ mlir::Attribute rewriteAttribute(const mlir::TypeConverter &tc, return llvm::TypeSwitch<mlir::Attribute, mlir::Attribute>(attr) // These attributes just have a type, so convert just the type. - .Case<cir::ZeroAttr>([tc](cir::ZeroAttr za) { + .Case<cir::ZeroAttr>([&tc](cir::ZeroAttr za) { return cir::ZeroAttr::get(tc.convertType(za.getType())); }) - .Case<cir::PoisonAttr>([tc](cir::PoisonAttr pa) { + .Case<cir::PoisonAttr>([&tc](cir::PoisonAttr pa) { return cir::PoisonAttr::get(tc.convertType(pa.getType())); }) - .Case<cir::UndefAttr>([tc](cir::UndefAttr uda) { + .Case<cir::UndefAttr>([&tc](cir::UndefAttr uda) { return cir::UndefAttr::get(tc.convertType(uda.getType())); }) - .Case<mlir::TypeAttr>([tc](mlir::TypeAttr ta) { + .Case<mlir::TypeAttr>([&tc](mlir::TypeAttr ta) { return mlir::TypeAttr::get(tc.convertType(ta.getValue())); }) - .Case<cir::ConstPtrAttr>([tc](cir::ConstPtrAttr cpa) { + .Case<cir::ConstPtrAttr>([&tc](cir::ConstPtrAttr cpa) { return cir::ConstPtrAttr::get(tc.convertType(cpa.getType()), cpa.getValue()); }) - .Case<cir::CXXCtorAttr>([tc](cir::CXXCtorAttr ca) { + .Case<cir::CXXCtorAttr>([&tc](cir::CXXCtorAttr ca) { return cir::CXXCtorAttr::get(tc.convertType(ca.getType()), ca.getCtorKind(), ca.getIsTrivial()); }) - .Case<cir::CXXDtorAttr>([tc](cir::CXXDtorAttr da) { + .Case<cir::CXXDtorAttr>([&tc](cir::CXXDtorAttr da) { return cir::CXXDtorAttr::get(tc.convertType(da.getType()), da.getIsTrivial()); }) - .Case<cir::CXXAssignAttr>([tc](cir::CXXAssignAttr aa) { + .Case<cir::CXXAssignAttr>([&tc](cir::CXXAssignAttr aa) { return cir::CXXAssignAttr::get(tc.convertType(aa.getType()), aa.getAssignKind(), aa.getIsTrivial()); }) // Collection attributes need to transform all of the attributes inside of // them. - .Case<mlir::ArrayAttr>([tc, ctx](mlir::ArrayAttr array) { + .Case<mlir::ArrayAttr>([&tc, ctx](mlir::ArrayAttr array) { llvm::SmallVector<mlir::Attribute> elts; for (mlir::Attribute a : array.getValue()) elts.push_back(rewriteAttribute(tc, ctx, a)); return mlir::ArrayAttr::get(ctx, elts); }) - .Case<mlir::DictionaryAttr>([tc, ctx](mlir::DictionaryAttr dict) { + .Case<mlir::DictionaryAttr>([&tc, ctx](mlir::DictionaryAttr dict) { llvm::SmallVector<mlir::NamedAttribute> elts; for (mlir::NamedAttribute na : dict.getValue()) elts.emplace_back(na.getName(), @@ -198,31 +196,32 @@ mlir::Attribute rewriteAttribute(const mlir::TypeConverter &tc, return mlir::DictionaryAttr::get(ctx, elts); }) // These attributes have sub-attributes that need converting too. - .Case<cir::ConstArrayAttr>([tc, ctx](cir::ConstArrayAttr array) { + .Case<cir::ConstArrayAttr>([&tc, ctx](cir::ConstArrayAttr array) { return cir::ConstArrayAttr::get( ctx, tc.convertType(array.getType()), rewriteAttribute(tc, ctx, array.getElts()), array.getTrailingZerosNum()); }) - .Case<cir::GlobalViewAttr>([tc, ctx](cir::GlobalViewAttr gva) { + .Case<cir::GlobalViewAttr>([&tc, ctx](cir::GlobalViewAttr gva) { return cir::GlobalViewAttr::get( tc.convertType(gva.getType()), gva.getSymbol(), mlir::cast<mlir::ArrayAttr>( rewriteAttribute(tc, ctx, gva.getIndices()))); }) - .Case<cir::VTableAttr>([tc, ctx](cir::VTableAttr vta) { + .Case<cir::VTableAttr>([&tc, ctx](cir::VTableAttr vta) { return cir::VTableAttr::get( tc.convertType(vta.getType()), mlir::cast<mlir::ArrayAttr>( rewriteAttribute(tc, ctx, vta.getData()))); }) - .Case<cir::TypeInfoAttr>([tc, ctx](cir::TypeInfoAttr tia) { + .Case<cir::TypeInfoAttr>([&tc, ctx](cir::TypeInfoAttr tia) { return cir::TypeInfoAttr::get( tc.convertType(tia.getType()), mlir::cast<mlir::ArrayAttr>( rewriteAttribute(tc, ctx, tia.getData()))); }) - .Case<cir::DynamicCastInfoAttr>([tc, ctx](cir::DynamicCastInfoAttr dcia) { + .Case<cir::DynamicCastInfoAttr>([&tc, + ctx](cir::DynamicCastInfoAttr dcia) { return cir::DynamicCastInfoAttr::get( mlir::cast<cir::GlobalViewAttr>( rewriteAttribute(tc, ctx, dcia.getSrcRtti())), @@ -230,7 +229,7 @@ mlir::Attribute rewriteAttribute(const mlir::TypeConverter &tc, rewriteAttribute(tc, ctx, dcia.getDestRtti())), dcia.getRuntimeFunc(), dcia.getBadCastFunc(), dcia.getOffsetHint()); }) - .Case<cir::ConstRecordAttr>([tc, ctx](cir::ConstRecordAttr cra) { + .Case<cir::ConstRecordAttr>([&tc, ctx](cir::ConstRecordAttr cra) { return cir::ConstRecordAttr::get( ctx, tc.convertType(cra.getType()), mlir::cast<mlir::ArrayAttr>( @@ -356,7 +355,7 @@ mlir::LogicalResult CIRCastOpABILowering::matchAndRewrite( if (mlir::isa<cir::DataMemberType>(srcTy)) loweredResult = lowerModule->getCXXABI().lowerDataMemberBitcast( op, destTy, adaptor.getSrc(), rewriter); - else if (mlir::isa<cir::MethodType>(srcTy)) + else loweredResult = lowerModule->getCXXABI().lowerMethodBitcast( op, destTy, adaptor.getSrc(), rewriter); rewriter.replaceOp(op, loweredResult); @@ -367,7 +366,7 @@ mlir::LogicalResult CIRCastOpABILowering::matchAndRewrite( if (mlir::isa<cir::DataMemberType>(srcTy)) loweredResult = lowerModule->getCXXABI().lowerDataMemberToBoolCast( op, adaptor.getSrc(), rewriter); - else if (mlir::isa<cir::MethodType>(srcTy)) + else loweredResult = lowerModule->getCXXABI().lowerMethodToBoolCast( op, adaptor.getSrc(), rewriter); rewriter.replaceOp(op, loweredResult); @@ -793,9 +792,11 @@ class CIRABITypeConverter : public mlir::TypeConverter { type.getContext(), convertRecordMemberTypes(type), type.getPacked(), type.getPadded(), type.getKind()); + assert(!type.isIncomplete() || type.getMembers().empty()); + // If the type has already been converted, we can just return, since there - // is nothing to do. Also, if is incomplete, it can't have invalid members! - // So we can skip transforming it. + // is nothing to do. Also, if it is incomplete, it can't have invalid + // members! So we can skip transforming it. if (type.isIncomplete() || type.isABIConvertedRecord()) return type; @@ -901,9 +902,8 @@ populateCXXABIConversionTarget(mlir::ConversionTarget &target, if (!typeConverter.isLegal(op)) return false; - bool attrs = std::all_of( - op->getAttrs().begin(), op->getAttrs().end(), - [&typeConverter](const mlir::NamedAttribute &a) { + bool attrs = llvm::all_of( + op->getAttrs(), [&typeConverter](const mlir::NamedAttribute &a) { return isCXXABIAttributeLegal(typeConverter, a.getValue()); }); @@ -919,9 +919,8 @@ populateCXXABIConversionTarget(mlir::ConversionTarget &target, if (!typeConverter.isLegal(op)) return false; - bool attrs = std::all_of( - op->getAttrs().begin(), op->getAttrs().end(), - [&typeConverter](const mlir::NamedAttribute &a) { + bool attrs = llvm::all_of( + op->getAttrs(), [&typeConverter](const mlir::NamedAttribute &a) { return isCXXABIAttributeLegal(typeConverter, a.getValue()); }); @@ -934,11 +933,10 @@ populateCXXABIConversionTarget(mlir::ConversionTarget &target, // Some CIR ops needs special checking for legality target.addDynamicallyLegalOp<cir::FuncOp>([&typeConverter](cir::FuncOp op) { - bool attrs = std::all_of(op->getAttrs().begin(), op->getAttrs().end(), - [&typeConverter](const mlir::NamedAttribute &a) { - return isCXXABIAttributeLegal(typeConverter, - a.getValue()); - }); + bool attrs = llvm::all_of( + op->getAttrs(), [&typeConverter](const mlir::NamedAttribute &a) { + return isCXXABIAttributeLegal(typeConverter, a.getValue()); + }); return attrs && typeConverter.isLegal(op.getFunctionType()); }); >From 4801a8197972dda62f7da9d7e6e7444d2b84d4cd Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 18 Mar 2026 15:19:04 -0700 Subject: [PATCH 4/6] Change the 'always legal'test to use tablegen per andy's suggestion --- .../include/clang/CIR/Dialect/IR/CIRAttrs.td | 21 +++++++++++++++++++ .../clang/CIR/Dialect/IR/CIRCUDAAttrs.td | 2 ++ .../CIR/Dialect/Transforms/CXXABILowering.cpp | 20 ++++++++---------- clang/utils/TableGen/CIRLoweringEmitter.cpp | 20 ++++++++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td index 16d32802d7c2c..439dd55cf2ecd 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td @@ -29,6 +29,11 @@ include "clang/CIR/Interfaces/ASTAttrInterfaces.td" class CIR_Attr<string name, string attrMnemonic, list<Trait> traits = []> : AttrDef<CIR_Dialect, name, traits> { let mnemonic = attrMnemonic; + // Setting this to 0 will make the CXXABILowering always consider this + // attribute 'legal', so that it won't require any transformation. This should + // not be set to 0 if this type has any ability to have a 'record' type, + // member type, or method type. + bit canHaveIllegalCXXABIType = 1; } class CIR_TypedAttr<string name, string attrMnemonic, list<Trait> traits = []> @@ -138,6 +143,7 @@ def CIR_OptInfoAttr : CIR_Attr<"OptInfo", "opt_info"> { `<` struct(params) `>` }]; let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -163,6 +169,7 @@ def CIR_BoolAttr : CIR_Attr<"Bool", "bool", [TypedAttrInterface]> { let assemblyFormat = [{ `<` $value `>` }]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -265,6 +272,7 @@ def CIR_IntAttr : CIR_Attr<"Int", "int", [TypedAttrInterface]> { }]; let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -299,6 +307,7 @@ def CIR_FPAttr : CIR_Attr<"FP", "fp", [TypedAttrInterface]> { }]; let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } @@ -381,6 +390,7 @@ def CIR_ConstVectorAttr : CIR_Attr<"ConstVector", "const_vector", [ // Enable verifier. let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -857,6 +867,7 @@ def CIR_TargetAddressSpaceAttr : CIR_Attr< "TargetAddressSpace", let parameters = (ins "unsigned":$value); let assemblyFormat = "`<` `target` `<` $value `>` `>`"; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -902,6 +913,7 @@ def CIR_ConstComplexAttr : CIR_Attr<"ConstComplex", "const_complex", [ let assemblyFormat = [{ `<` qualified($real) `,` qualified($imag) `>` }]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -978,6 +990,7 @@ class CIR_GlobalCtorDtor<string name, string attrMnemonic> return 65535; } }]; + let canHaveIllegalCXXABIType = 0; } def CIR_GlobalCtorAttr : CIR_GlobalCtorDtor<"Ctor", "ctor"> { @@ -1169,6 +1182,7 @@ def CIR_BitfieldInfoAttr : CIR_Attr<"BitfieldInfo", "bitfield_info"> { size, offset, is_signed); }]> ]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1201,6 +1215,7 @@ def CIR_AddressPointAttr : CIR_Attr<"AddressPoint", "address_point"> { let assemblyFormat = [{ `<` struct($index, $offset) `>` }]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1271,12 +1286,14 @@ def CIR_InlineKind : CIR_I32EnumAttr<"InlineKind", "inlineKind", [ // Represents the catch_all region. def CIR_CatchAllAttr : CIR_UnitAttr<"CatchAll", "all"> { let storageType = [{ CatchAllAttr }]; + let canHaveIllegalCXXABIType = 0; } // Represents the unwind region where unwind continues or // the program std::terminate's. def CIR_UnwindAttr : CIR_UnitAttr<"Unwind", "unwind"> { let storageType = [{ CatchUnwind }]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1302,6 +1319,7 @@ def CIR_BlockAddrInfoAttr : CIR_Attr<"BlockAddrInfo", "block_addr_info"> { mlir::StringAttr::get($_ctxt, label_name)); }]> ]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1355,6 +1373,7 @@ def CIR_StaticLocalGuardAttr : CIR_Attr<"StaticLocalGuard", }]; let parameters = (ins "mlir::StringAttr":$name); let assemblyFormat = "`<` $name `>`"; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1379,6 +1398,7 @@ def CIR_UsualDeleteParamsAttr let assemblyFormat = [{ `<` struct($size, $alignment, $type_aware_delete, $destroying_delete) `>` }]; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// @@ -1415,6 +1435,7 @@ class CIR_AST<string name, string prefix, list<Trait> traits = []> // Nothing to print besides the mnemonics. } }]; + let canHaveIllegalCXXABIType = 0; } def CIR_ASTVarDeclAttr : CIR_AST<"VarDecl", "var.decl", [ diff --git a/clang/include/clang/CIR/Dialect/IR/CIRCUDAAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRCUDAAttrs.td index 257cf396abce7..5932db8323196 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRCUDAAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRCUDAAttrs.td @@ -34,6 +34,7 @@ def CIR_CUDAKernelNameAttr : CIR_Attr<"CUDAKernelName", "cu.kernel_name"> { let parameters = (ins "std::string":$kernel_name); let assemblyFormat = "`<` $kernel_name `>`"; + let canHaveIllegalCXXABIType = 0; } def CUDAExternallyInitializedAttr : CIR_Attr<"CUDAExternallyInitialized", @@ -47,6 +48,7 @@ def CUDAExternallyInitializedAttr : CIR_Attr<"CUDAExternallyInitialized", The attribute corresponds to the attribute on LLVM with the same name. }]; + let canHaveIllegalCXXABIType = 0; } #endif // CLANG_CIR_DIALECT_IR_CIRCUDAATTRS_TD diff --git a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp index 20c69414937ad..3a8fc0c41c08a 100644 --- a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp @@ -52,20 +52,18 @@ bool isCXXABIAttributeLegal(const mlir::TypeConverter &tc, // These attributes either don't contain a type, or don't contain a type that // can have a data member/method. - if (isa<cir::IntAttr, cir::BoolAttr, cir::SourceLanguageAttr, - cir::OptInfoAttr, cir::ConstVectorAttr, cir::FPAttr, - cir::CUDAKernelNameAttr, cir::VisibilityAttr, cir::GlobalCtorAttr, - cir::GlobalDtorAttr, cir::LangAddressSpaceAttr, - cir::TargetAddressSpaceAttr, cir::BitfieldInfoAttr, - cir::AddressPointAttr, cir::BlockAddrInfoAttr, - cir::StaticLocalGuardAttr, cir::UsualDeleteParamsAttr, - cir::ASTVarDeclAttr, cir::CUDAExternallyInitializedAttr, - cir::CleanupKindAttr, cir::CleanupKindAttr, cir::ConstComplexAttr, - cir::CatchAllAttr, cir::UnwindAttr, cir::BlockAddrInfoAttr, - mlir::DenseArrayAttr, mlir::FloatAttr, mlir::UnitAttr, + if (isa<mlir::DenseArrayAttr, mlir::FloatAttr, mlir::UnitAttr, mlir::StringAttr, mlir::IntegerAttr, mlir::SymbolRefAttr>(attr)) return true; + // Tablegen'ed always-legal attributes: + if (isa< +#define CXX_ABI_ALWAYS_LEGAL_ATTRS +#include "clang/CIR/Dialect/IR/CIRLowering.inc" +#undef CXX_ABI_ALWAYS_LEGAL_ATTRS + >(attr)) + return true; + // Data Member and method are ALWAYS illegal. if (isa<cir::DataMemberAttr, cir::MethodAttr>(attr)) return false; diff --git a/clang/utils/TableGen/CIRLoweringEmitter.cpp b/clang/utils/TableGen/CIRLoweringEmitter.cpp index 21c3caca7c66d..36417b6df8369 100644 --- a/clang/utils/TableGen/CIRLoweringEmitter.cpp +++ b/clang/utils/TableGen/CIRLoweringEmitter.cpp @@ -23,6 +23,7 @@ using namespace clang; namespace { std::vector<std::string> CXXABILoweringPatterns; std::vector<std::string> CXXABILoweringPatternsList; +std::vector<std::string> CXXABILoweringAttrAlwaysLegal; std::vector<std::string> LLVMLoweringPatterns; std::vector<std::string> LLVMLoweringPatternsList; @@ -216,6 +217,18 @@ void Generate(const Record *OpRecord) { LLVMLoweringPatternsList.push_back(std::move(PatternName)); } } + +void GenerateCIREnumAttrs(const Record *Record) { + std::string OpName = GetOpCppClassName(Record); + // EnumAttr is in a separate hierarchy, so we have to set these separately, as + // they never have an 'illegal' CXXABI type in them. + CXXABILoweringAttrAlwaysLegal.push_back("cir::" + OpName); +} +void GenerateCIRAttrs(const Record *Record) { + std::string OpName = GetOpCppClassName(Record); + if (!Record->getValueAsBit("canHaveIllegalCXXABIType")) + CXXABILoweringAttrAlwaysLegal.push_back("cir::" + OpName); +} } // namespace void clang::EmitCIRLowering(const llvm::RecordKeeper &RK, @@ -223,6 +236,10 @@ void clang::EmitCIRLowering(const llvm::RecordKeeper &RK, emitSourceFileHeader("Lowering patterns for CIR operations", OS); for (const auto *OpRecord : RK.getAllDerivedDefinitions("CIR_Op")) Generate(OpRecord); + for (const auto *OpRecord : RK.getAllDerivedDefinitions("EnumAttr")) + GenerateCIREnumAttrs(OpRecord); + for (const auto *OpRecord : RK.getAllDerivedDefinitions("CIR_Attr")) + GenerateCIRAttrs(OpRecord); OS << "#ifdef GET_ABI_LOWERING_PATTERNS\n" << llvm::join(CXXABILoweringPatterns, "\n") << "#endif\n\n"; @@ -233,4 +250,7 @@ void clang::EmitCIRLowering(const llvm::RecordKeeper &RK, << llvm::join(LLVMLoweringPatterns, "\n") << "#endif\n\n"; OS << "#ifdef GET_LLVM_LOWERING_PATTERNS_LIST\n" << llvm::join(LLVMLoweringPatternsList, ",\n") << "\n#endif\n\n"; + + OS << "#ifdef CXX_ABI_ALWAYS_LEGAL_ATTRS\n" + << llvm::join(CXXABILoweringAttrAlwaysLegal, ",\n") << "\n#endif\n\n"; } >From 7910b0de0e7419d7e9eead5b346fb7042a44c470 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 18 Mar 2026 15:35:35 -0700 Subject: [PATCH 5/6] Fixup 3 way by marking it 'always legal' the 3 way test also managed to have an operation that was only 'illegal' based on an attribute, and we weren't doing our conversion if only attributes were different. So this patch adds the logic to make sure we do that right too. However, the test would no longer produce that of course, since the attribute is marked 'legal', and I only discovered the problem because I typo'ed the CIRAttrs.td change here. --- clang/include/clang/CIR/Dialect/IR/CIRAttrs.td | 1 + clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td index e92018e031245..a98987db7b34d 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td +++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td @@ -664,6 +664,7 @@ def CIR_CmpThreeWayInfoAttr : CIR_Attr<"CmpThreeWayInfo", "cmp3way_info"> { }]; let genVerifyDecl = 1; + let canHaveIllegalCXXABIType = 0; } //===----------------------------------------------------------------------===// diff --git a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp index 3a8fc0c41c08a..1009ed32be24f 100644 --- a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp @@ -45,8 +45,8 @@ bool isCXXABIAttributeLegal(const mlir::TypeConverter &tc, if (!attr) return true; - // None of the OpenACC attributes contain a type of concern, so we can just - // treat them as legal. + // None of the OpenACC attributes contain a type of concern, so we can + // just treat them as legal. if (isa<mlir::acc::OpenACCDialect>(attr.getDialect())) return true; @@ -277,7 +277,12 @@ class CIRGenericCXXABILoweringPattern : public mlir::ConversionPattern { [typeConverter](mlir::Region ®ion) { return typeConverter->isLegal(®ion); }); - if (operandsAndResultsLegal && regionsLegal) { + bool attrsLegal = + llvm::all_of(op->getAttrs(), [typeConverter](mlir::NamedAttribute na) { + return isCXXABIAttributeLegal(*typeConverter, na.getValue()); + }); + + if (operandsAndResultsLegal && regionsLegal && attrsLegal) { // The operation does not have any CXXABI-dependent operands or results, // the match fails. return mlir::failure(); >From 5997d31e32917d970bd5fe05f01e023aad3eb4ac Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Wed, 18 Mar 2026 16:41:14 -0700 Subject: [PATCH 6/6] Also mark OMP attributes all legal --- clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp index 1009ed32be24f..25fb3a042dadb 100644 --- a/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp +++ b/clang/lib/CIR/Dialect/Transforms/CXXABILowering.cpp @@ -12,6 +12,7 @@ #include "TargetLowering/LowerModule.h" #include "mlir/Dialect/OpenACC/OpenACCOpsDialect.h.inc" +#include "mlir/Dialect/OpenMP/OpenMPOpsDialect.h.inc" #include "mlir/IR/PatternMatch.h" #include "mlir/Interfaces/DataLayoutInterfaces.h" #include "mlir/Pass/Pass.h" @@ -45,9 +46,10 @@ bool isCXXABIAttributeLegal(const mlir::TypeConverter &tc, if (!attr) return true; - // None of the OpenACC attributes contain a type of concern, so we can + // None of the OpenACC/OMP attributes contain a type of concern, so we can // just treat them as legal. - if (isa<mlir::acc::OpenACCDialect>(attr.getDialect())) + if (isa<mlir::acc::OpenACCDialect, mlir::omp::OpenMPDialect>( + attr.getDialect())) return true; // These attributes either don't contain a type, or don't contain a type that _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
