llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) <details> <summary>Changes</summary> This adds the support for implicit VTT arguments in constructors. --- Patch is 66.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/156521.diff 17 Files Affected: - (modified) clang/include/clang/CIR/MissingFeatures.h (-4) - (modified) clang/lib/CIR/CodeGen/CIRGenBuilder.h (+33) - (modified) clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp (+14) - (modified) clang/lib/CIR/CodeGen/CIRGenCXXABI.h (+79) - (modified) clang/lib/CIR/CodeGen/CIRGenCall.cpp (+12-8) - (modified) clang/lib/CIR/CodeGen/CIRGenClass.cpp (+143-7) - (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+1-1) - (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+20) - (modified) clang/lib/CIR/CodeGen/CIRGenItaniumCXXABI.cpp (+144-12) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+11-4) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+10) - (modified) clang/lib/CIR/CodeGen/CIRGenTypes.h (+2-1) - (modified) clang/lib/CIR/CodeGen/CIRGenVTables.cpp (+46) - (modified) clang/lib/CIR/CodeGen/CIRGenVTables.h (+21) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+1-2) - (modified) clang/test/CIR/CodeGen/vtt.cpp (+319) - (modified) clang/test/CIR/Lowering/vtt-addrpoint.cir (+1-1) ``````````diff diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 70e0abe30e416..85c18212f9abd 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -132,10 +132,8 @@ struct MissingFeatures { // RecordType static bool skippedLayout() { return false; } static bool astRecordDeclAttr() { return false; } - static bool cxxSupport() { return false; } static bool recordZeroInit() { return false; } static bool zeroSizeRecordMembers() { return false; } - static bool recordLayoutVirtualBases() { return false; } // Various handling of deferred processing in CIRGenModule. static bool cgmRelease() { return false; } @@ -148,7 +146,6 @@ struct MissingFeatures { static bool cxxabiUseARMMethodPtrABI() { return false; } static bool cxxabiUseARMGuardVarABI() { return false; } static bool cxxabiAppleARM64CXXABI() { return false; } - static bool cxxabiStructorImplicitParam() { return false; } static bool isDiscreteBitFieldABI() { return false; } // Address class @@ -229,7 +226,6 @@ struct MissingFeatures { static bool globalViewIndices() { return false; } static bool globalViewIntLowering() { return false; } static bool hip() { return false; } - static bool implicitConstructorArgs() { return false; } static bool incrementProfileCounter() { return false; } static bool innermostEHScope() { return false; } static bool insertBuiltinUnpredictable() { return false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h index 41207afd6a43b..5bf29fad2f3dd 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h +++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h @@ -287,6 +287,10 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy { cir::ConstantOp getUInt32(uint32_t c, mlir::Location loc) { return getConstantInt(loc, getUInt32Ty(), c); } + cir::ConstantOp getSInt64(uint64_t c, mlir::Location loc) { + cir::IntType sInt64Ty = getSInt64Ty(); + return create<cir::ConstantOp>(loc, cir::IntAttr::get(sInt64Ty, c)); + } // Creates constant nullptr for pointer type ty. cir::ConstantOp getNullPtr(mlir::Type ty, mlir::Location loc) { @@ -359,6 +363,18 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy { return Address(baseAddr, destType, addr.getAlignment()); } + mlir::Value createVTTAddrPoint(mlir::Location loc, mlir::Type retTy, + mlir::Value addr, uint64_t offset) { + return cir::VTTAddrPointOp::create(*this, loc, retTy, + mlir::FlatSymbolRefAttr{}, addr, offset); + } + + mlir::Value createVTTAddrPoint(mlir::Location loc, mlir::Type retTy, + mlir::FlatSymbolRefAttr sym, uint64_t offset) { + return cir::VTTAddrPointOp::create(*this, loc, retTy, sym, mlir::Value{}, + offset); + } + /// Cast the element type of the given address to a different type, /// preserving information like the alignment. Address createElementBitCast(mlir::Location loc, Address addr, @@ -379,6 +395,23 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy { /*mem_order=*/cir::MemOrderAttr{}); } + cir::LoadOp createAlignedLoad(mlir::Location loc, mlir::Type ty, + mlir::Value ptr, llvm::MaybeAlign align) { + if (ty != mlir::cast<cir::PointerType>(ptr.getType()).getPointee()) + ptr = createPtrBitcast(ptr, ty); + uint64_t alignment = align ? align->value() : 0; + mlir::IntegerAttr alignAttr = getAlignmentAttr(alignment); + return create<cir::LoadOp>(loc, ptr, /*isDeref=*/false, + /*isVolatile=*/false, alignAttr, + /*mem_order=*/cir::MemOrderAttr{}); + } + + cir::LoadOp + createAlignedLoad(mlir::Location loc, mlir::Type ty, mlir::Value ptr, + clang::CharUnits align = clang::CharUnits::One()) { + return createAlignedLoad(loc, ty, ptr, align.getAsAlign()); + } + cir::StoreOp createStore(mlir::Location loc, mlir::Value val, Address dst, bool isVolatile = false, mlir::IntegerAttr align = {}, diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp b/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp index 33b812ac81f6e..5f1faabde22a5 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.cpp @@ -22,6 +22,20 @@ using namespace clang::CIRGen; CIRGenCXXABI::~CIRGenCXXABI() {} +CIRGenCXXABI::AddedStructorArgCounts CIRGenCXXABI::addImplicitConstructorArgs( + CIRGenFunction &cgf, const CXXConstructorDecl *d, CXXCtorType type, + bool forVirtualBase, bool delegating, CallArgList &args) { + AddedStructorArgs addedArgs = + getImplicitConstructorArgs(cgf, d, type, forVirtualBase, delegating); + for (auto [idx, prefixArg] : llvm::enumerate(addedArgs.prefix)) + args.insert(args.begin() + 1 + idx, + CallArg(RValue::get(prefixArg.value), prefixArg.type)); + for (const auto &arg : addedArgs.suffix) + args.add(RValue::get(arg.value), arg.type); + return AddedStructorArgCounts(addedArgs.prefix.size(), + addedArgs.suffix.size()); +} + void CIRGenCXXABI::buildThisParam(CIRGenFunction &cgf, FunctionArgList ¶ms) { const auto *md = cast<CXXMethodDecl>(cgf.curGD.getDecl()); diff --git a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h index 32c4ef28532ea..ae922599809b8 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCXXABI.h +++ b/clang/lib/CIR/CodeGen/CIRGenCXXABI.h @@ -47,11 +47,67 @@ class CIRGenCXXABI { /// constructor/destructor Decl. virtual void emitCXXStructor(clang::GlobalDecl gd) = 0; + virtual mlir::Value + getVirtualBaseClassOffset(mlir::Location loc, CIRGenFunction &cgf, + Address thisAddr, const CXXRecordDecl *classDecl, + const CXXRecordDecl *baseClassDecl) = 0; + public: + /// Similar to AddedStructorArgs, but only notes the number of additional + /// arguments. + struct AddedStructorArgCounts { + unsigned prefix = 0; + unsigned suffix = 0; + AddedStructorArgCounts() = default; + AddedStructorArgCounts(unsigned p, unsigned s) : prefix(p), suffix(s) {} + static AddedStructorArgCounts withPrefix(unsigned n) { return {n, 0}; } + static AddedStructorArgCounts withSuffix(unsigned n) { return {0, n}; } + }; + + /// Additional implicit arguments to add to the beginning (Prefix) and end + /// (Suffix) of a constructor / destructor arg list. + /// + /// Note that Prefix should actually be inserted *after* the first existing + /// arg; `this` arguments always come first. + struct AddedStructorArgs { + struct Arg { + mlir::Value value; + QualType type; + }; + llvm::SmallVector<Arg, 1> prefix; + llvm::SmallVector<Arg, 1> suffix; + AddedStructorArgs() = default; + AddedStructorArgs(llvm::SmallVector<Arg, 1> p, llvm::SmallVector<Arg, 1> s) + : prefix(std::move(p)), suffix(std::move(s)) {} + static AddedStructorArgs withPrefix(llvm::SmallVector<Arg, 1> args) { + return {std::move(args), {}}; + } + static AddedStructorArgs withSuffix(llvm::SmallVector<Arg, 1> args) { + return {{}, std::move(args)}; + } + }; + + /// Build the signature of the given constructor or destructor vairant by + /// adding any required parameters. For convenience, ArgTys has been + /// initialized with the type of 'this'. + virtual AddedStructorArgCounts + buildStructorSignature(GlobalDecl gd, + llvm::SmallVectorImpl<CanQualType> &argTys) = 0; + + AddedStructorArgCounts + addImplicitConstructorArgs(CIRGenFunction &cgf, const CXXConstructorDecl *d, + CXXCtorType type, bool forVirtualBase, + bool delegating, CallArgList &args); + clang::ImplicitParamDecl *getThisDecl(CIRGenFunction &cgf) { return cgf.cxxabiThisDecl; } + virtual AddedStructorArgs + getImplicitConstructorArgs(CIRGenFunction &cgf, const CXXConstructorDecl *d, + CXXCtorType type, bool forVirtualBase, + bool delegating) = 0; + /// Emit the ABI-specific prolog for the function virtual void emitInstanceFunctionProlog(SourceLocation loc, CIRGenFunction &cgf) = 0; @@ -144,6 +200,17 @@ class CIRGenCXXABI { CIRGenFunction &cgf, const CXXRecordDecl *vtableClass, BaseSubobject base, const CXXRecordDecl *nearestVBase) = 0; + /// Insert any ABI-specific implicit parameters into the parameter list for a + /// function. This generally involves extra data for constructors and + /// destructors. + /// + /// ABIs may also choose to override the return type, which has been + /// initialized with the type of 'this' if HasThisReturn(CGF.CurGD) is true or + /// the formal return type of the function otherwise. + virtual void addImplicitStructorParams(CIRGenFunction &cgf, + clang::QualType &resTy, + FunctionArgList ¶ms) = 0; + /// Checks if ABI requires to initialize vptrs for given dynamic class. virtual bool doStructorsInitializeVPtrs(const clang::CXXRecordDecl *vtableClass) = 0; @@ -162,6 +229,18 @@ class CIRGenCXXABI { /// Gets the mangle context. clang::MangleContext &getMangleContext() { return *mangleContext; } + + clang::ImplicitParamDecl *&getStructorImplicitParamDecl(CIRGenFunction &cgf) { + return cgf.cxxStructorImplicitParamDecl; + } + + mlir::Value getStructorImplicitParamValue(CIRGenFunction &cgf) { + return cgf.cxxStructorImplicitParamValue; + } + + void setStructorImplicitParamValue(CIRGenFunction &cgf, mlir::Value val) { + cgf.cxxStructorImplicitParamValue = val; + } }; /// Creates and Itanium-family ABI diff --git a/clang/lib/CIR/CodeGen/CIRGenCall.cpp b/clang/lib/CIR/CodeGen/CIRGenCall.cpp index 25859885296fa..2970b369a85d0 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCall.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCall.cpp @@ -197,7 +197,10 @@ CIRGenTypes::arrangeCXXStructorDeclaration(GlobalDecl gd) { if (passParams) appendParameterTypes(*this, argTypes, fpt); - assert(!cir::MissingFeatures::implicitConstructorArgs()); + // The structor signature may include implicit parameters. + [[maybe_unused]] CIRGenCXXABI::AddedStructorArgCounts addedArgs = + theCXXABI.buildStructorSignature(gd, argTypes); + assert(!cir::MissingFeatures::opCallCIRGenFuncInfoExtParamInfo()); RequiredArgs required = (passParams && md->isVariadic() ? RequiredArgs(argTypes.size()) @@ -324,26 +327,27 @@ arrangeFreeFunctionLikeCall(CIRGenTypes &cgt, CIRGenModule &cgm, /// Arrange a call to a C++ method, passing the given arguments. /// +/// extraPrefixArgs is the number of ABI-specific args passed after the `this` +/// parameter. /// passProtoArgs indicates whether `args` has args for the parameters in the /// given CXXConstructorDecl. const CIRGenFunctionInfo &CIRGenTypes::arrangeCXXConstructorCall( const CallArgList &args, const CXXConstructorDecl *d, CXXCtorType ctorKind, - bool passProtoArgs) { + unsigned extraPrefixArgs, unsigned extraSuffixArgs, bool passProtoArgs) { // FIXME: Kill copy. llvm::SmallVector<CanQualType, 16> argTypes; for (const auto &arg : args) argTypes.push_back(astContext.getCanonicalParamType(arg.ty)); - assert(!cir::MissingFeatures::implicitConstructorArgs()); // +1 for implicit this, which should always be args[0] - unsigned totalPrefixArgs = 1; + unsigned totalPrefixArgs = 1 + extraPrefixArgs; CanQual<FunctionProtoType> fpt = getFormalType(d); - RequiredArgs required = - passProtoArgs - ? RequiredArgs::getFromProtoWithExtraSlots(fpt, totalPrefixArgs) - : RequiredArgs::All; + RequiredArgs required = passProtoArgs + ? RequiredArgs::getFromProtoWithExtraSlots( + fpt, totalPrefixArgs + extraSuffixArgs) + : RequiredArgs::All; GlobalDecl gd(d, ctorKind); if (theCXXABI.hasThisReturn(gd)) diff --git a/clang/lib/CIR/CodeGen/CIRGenClass.cpp b/clang/lib/CIR/CodeGen/CIRGenClass.cpp index 9a27932c12dff..5eff039928adc 100644 --- a/clang/lib/CIR/CodeGen/CIRGenClass.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenClass.cpp @@ -316,6 +316,7 @@ static Address applyNonVirtualAndVirtualOffset( assert(!nonVirtualOffset.isZero() || virtualOffset != nullptr); // Compute the offset from the static and dynamic components. + mlir::Value baseOffset; if (!nonVirtualOffset.isZero()) { if (virtualOffset) { cgf.cgm.errorNYI( @@ -329,10 +330,35 @@ static Address applyNonVirtualAndVirtualOffset( loc, addr, baseValueTy, nonVirtualOffset.getQuantity(), assumeNotNull); } + } else { + baseOffset = virtualOffset; + } + + // Apply the base offset. cir.ptr_stride adjusts by a number of elements, + // not bytes. So the pointer must be cast to a byte pointer and back. + + mlir::Value ptr = addr.getPointer(); + mlir::Type charPtrType = cgf.cgm.UInt8PtrTy; + mlir::Value charPtr = + cgf.getBuilder().createCast(cir::CastKind::bitcast, ptr, charPtrType); + mlir::Value adjusted = cir::PtrStrideOp::create( + cgf.getBuilder(), loc, charPtrType, charPtr, baseOffset); + ptr = cgf.getBuilder().createCast(cir::CastKind::bitcast, adjusted, + ptr.getType()); + + // If we have a virtual component, the alignment of the result will + // be relative only to the known alignment of that vbase. + CharUnits alignment; + if (virtualOffset) { + assert(nearestVBase && "virtual offset without vbase?"); + alignment = cgf.cgm.getVBaseAlignment(addr.getAlignment(), derivedClass, + nearestVBase); + } else { + alignment = addr.getAlignment(); } + alignment = alignment.alignmentAtOffset(nonVirtualOffset); - cgf.cgm.errorNYI(loc, "applyNonVirtualAndVirtualOffset: virtual offset"); - return Address::invalid(); + return Address(ptr, alignment); } void CIRGenFunction::initializeVTablePointer(mlir::Location loc, @@ -351,7 +377,11 @@ void CIRGenFunction::initializeVTablePointer(mlir::Location loc, mlir::Type baseValueTy; if (cgm.getCXXABI().isVirtualOffsetNeededForVTableField(*this, vptr)) { - cgm.errorNYI(loc, "initializeVTablePointer: virtual offset for vtable"); + // We need to use the virtual base offset offset because the virtual base + // might have a different offset in the most derived class. + virtualOffset = cgm.getCXXABI().getVirtualBaseClassOffset( + loc, *this, loadCXXThisAddress(), vptr.vtableClass, vptr.nearestVBase); + nonVirtualOffset = vptr.offsetFromNearestVBase; } else { // We can just use the base offset in the complete class. nonVirtualOffset = vptr.base.getBaseOffset(); @@ -447,14 +477,14 @@ void CIRGenFunction::getVTablePointers(BaseSubobject base, const ASTRecordLayout &layout = getContext().getASTRecordLayout(vtableClass); - nextBaseDecl = nearestVBase; + nextBaseDecl = baseDecl; baseOffset = layout.getVBaseClassOffset(baseDecl); baseOffsetFromNearestVBase = CharUnits::Zero(); baseDeclIsNonVirtualPrimaryBase = false; } else { const ASTRecordLayout &layout = getContext().getASTRecordLayout(rd); - nextBaseDecl = baseDecl; + nextBaseDecl = nearestVBase; baseOffset = base.getBaseOffset() + layout.getBaseClassOffset(baseDecl); baseOffsetFromNearestVBase = offsetFromNearestVBase + layout.getBaseClassOffset(baseDecl); @@ -511,6 +541,64 @@ void CIRGenFunction::emitInitializerForField(FieldDecl *field, LValue lhs, assert(!cir::MissingFeatures::requiresCleanups()); } +CharUnits +CIRGenModule::getDynamicOffsetAlignment(CharUnits actualBaseAlign, + const CXXRecordDecl *baseDecl, + CharUnits expectedTargetAlign) { + // If the base is an incomplete type (which is, alas, possible with + // member pointers), be pessimistic. + if (!baseDecl->isCompleteDefinition()) + return std::min(actualBaseAlign, expectedTargetAlign); + + const ASTRecordLayout &baseLayout = + getASTContext().getASTRecordLayout(baseDecl); + CharUnits expectedBaseAlign = baseLayout.getNonVirtualAlignment(); + + // If the class is properly aligned, assume the target offset is, too. + // + // This actually isn't necessarily the right thing to do --- if the + // class is a complete object, but it's only properly aligned for a + // base subobject, then the alignments of things relative to it are + // probably off as well. (Note that this requires the alignment of + // the target to be greater than the NV alignment of the derived + // class.) + // + // However, our approach to this kind of under-alignment can only + // ever be best effort; after all, we're never going to propagate + // alignments through variables or parameters. Note, in particular, + // that constructing a polymorphic type in an address that's less + // than pointer-aligned will generally trap in the constructor, + // unless we someday add some sort of attribute to change the + // assumed alignment of 'this'. So our goal here is pretty much + // just to allow the user to explicitly say that a pointer is + // under-aligned and then safely access its fields and vtables. + if (actualBaseAlign >= expectedBaseAlign) + return expectedTargetAlign; + + // Otherwise, we might be offset by an arbitrary multiple of the + // actual alignment. The correct adjustment is to take the min of + // the two alignments. + return std::min(actualBaseAlign, expectedTargetAlign); +} + +/// Return the best known alignment for a pointer to a virtual base, +/// given the alignment of a pointer to the derived class. +clang::CharUnits +CIRGenModule::getVBaseAlignment(CharUnits actualDerivedAlign, + const CXXRecordDecl *derivedClass, + const CXXRecordDecl *vbaseClass) { + // The basic idea here is that an underaligned derived pointer might + // indicate an underaligned base pointer. + + assert(vbaseClass->isCompleteDefinition()); + const ASTRecordLayout &baseLayout = + getASTContext().getASTRecordLayout(vbaseClass); + CharUnits expectedVBaseAlign = baseLayout.getNonVirtualAlignment(); + + return getDynamicOffsetAlignment(actualDerivedAlign, derivedClass, + expectedVBaseAlign); +} + /// Emit a loop to call a particular constructor for each of several members /// of an array. /// @@ -723,6 +811,52 @@ void CIRGenFunction::emitCXXDestructorCall(const CXXDestructorDecl *dd, delegating, thisAddr, thisTy); } +mlir::Value CIRGenFunction::getVTTParameter(GlobalDecl gd, bool forVirtualBase, + bool delegating) { + if (!cgm.getCXXABI().needsVTTParameter(gd)) + return nullptr; + + const CXXRecordDecl *rd = cast<CXXMethodDecl>(curFuncDecl)->getParent(); + const CXXRecordDecl *base = cast<CXXMethodDecl>(gd.getDecl())->getParent(); + + uint64_t subVTTIndex; + + if (delegating) { + cgm.errorNYI(rd->getSourceRange(), + "getVTTParameter: delegating constructor"); + return {}; + } else if (rd == base) { + // If the record matches the base, this is the complete ctor/dtor + // variant calling the base variant in a class with virtual bases. + assert(!cgm.getCXXABI().needsVTTParameter(curGD) && + "doing no-op VTT offset in base dtor/ctor?"); + assert(!forVirtualBase && "Can't have same class as virtual base!"); + subVTTIndex = 0; + } else { + const ASTRecordLayout &layout = getContext().getASTRecordLayout(rd); + CharUnits baseOffset = forVirtualBase ? layout.getVBaseClassOffset(base) + : layout.getBaseClassOffset(base); + + subVTTIndex = + cgm.getVTables().getSubVTTIndex(rd, BaseSubobject(base, baseOffset)); + assert(subVTTIndex != 0 && "Sub-VTT index must be greater than z... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/156521 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
