diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp index e4836c850a157..30021794a0bb3 100644 --- a/clang/lib/CodeGen/CodeGenTypes.cpp +++ b/clang/lib/CodeGen/CodeGenTypes.cpp @@ -125,93 +125,9 @@ bool CodeGenTypes::isRecordLayoutComplete(const Type *Ty) const { return I != RecordDeclTypes.end() && !I->second->isOpaque(); } -static bool -isSafeToConvert(QualType T, CodeGenTypes &CGT, - llvm::SmallPtrSet &AlreadyChecked); - - -/// isSafeToConvert - Return true if it is safe to convert the specified record -/// decl to IR and lay it out, false if doing so would cause us to get into a -/// recursive compilation mess. -static bool -isSafeToConvert(const RecordDecl *RD, CodeGenTypes &CGT, - llvm::SmallPtrSet &AlreadyChecked) { - // If we have already checked this type (maybe the same type is used by-value - // multiple times in multiple structure fields, don't check again. - if (!AlreadyChecked.insert(RD).second) - return true; - - const Type *Key = CGT.getContext().getTagDeclType(RD).getTypePtr(); - - // If this type is already laid out, converting it is a noop. - if (CGT.isRecordLayoutComplete(Key)) return true; - - // If this type is currently being laid out, we can't recursively compile it. - if (CGT.isRecordBeingLaidOut(Key)) - return false; - - // If this type would require laying out bases that are currently being laid - // out, don't do it. This includes virtual base classes which get laid out - // when a class is translated, even though they aren't embedded by-value into - // the class. - if (const CXXRecordDecl *CRD = dyn_cast(RD)) { - for (const auto &I : CRD->bases()) - if (!isSafeToConvert(I.getType()->castAs()->getDecl(), CGT, - AlreadyChecked)) - return false; - } - - // If this type would require laying out members that are currently being laid - // out, don't do it. - for (const auto *I : RD->fields()) - if (!isSafeToConvert(I->getType(), CGT, AlreadyChecked)) - return false; - - // If there are no problems, lets do it. - return true; -} - -/// isSafeToConvert - Return true if it is safe to convert this field type, -/// which requires the structure elements contained by-value to all be -/// recursively safe to convert. -static bool -isSafeToConvert(QualType T, CodeGenTypes &CGT, - llvm::SmallPtrSet &AlreadyChecked) { - // Strip off atomic type sugar. - if (const auto *AT = T->getAs()) - T = AT->getValueType(); - - // If this is a record, check it. - if (const auto *RT = T->getAs()) - return isSafeToConvert(RT->getDecl(), CGT, AlreadyChecked); - - // If this is an array, check the elements, which are embedded inline. - if (const auto *AT = CGT.getContext().getAsArrayType(T)) - return isSafeToConvert(AT->getElementType(), CGT, AlreadyChecked); - - // Otherwise, there is no concern about transforming this. We only care about - // things that are contained by-value in a structure that can have another - // structure as a member. - return true; -} - - -/// isSafeToConvert - Return true if it is safe to convert the specified record -/// decl to IR and lay it out, false if doing so would cause us to get into a -/// recursive compilation mess. -static bool isSafeToConvert(const RecordDecl *RD, CodeGenTypes &CGT) { - // If no structs are being laid out, we can certainly do this one. - if (CGT.noRecordsBeingLaidOut()) return true; - - llvm::SmallPtrSet AlreadyChecked; - return isSafeToConvert(RD, CGT, AlreadyChecked); -} - /// isFuncParamTypeConvertible - Return true if the specified type in a /// function parameter or result position can be converted to an IR type at this -/// point. This boils down to being whether it is complete, as well as whether -/// we've temporarily deferred expanding the type because we're in a recursive -/// context. +/// point. This boils down to being whether it is complete. bool CodeGenTypes::isFuncParamTypeConvertible(QualType Ty) { // Some ABIs cannot have their member pointers represented in IR unless // certain circumstances have been reached. @@ -223,21 +139,7 @@ bool CodeGenTypes::isFuncParamTypeConvertible(QualType Ty) { if (!TT) return true; // Incomplete types cannot be converted. - if (TT->isIncompleteType()) - return false; - - // If this is an enum, then it is always safe to convert. - const RecordType *RT = dyn_cast(TT); - if (!RT) return true; - - // Otherwise, we have to be careful. If it is a struct that we're in the - // process of expanding, then we can't convert the function type. That's ok - // though because we must be in a pointer context under the struct, so we can - // just convert it to a dummy type. - // - // We decide this by checking whether ConvertRecordDeclType returns us an - // opaque type for a struct that we know is defined. - return isSafeToConvert(RT->getDecl(), *this); + return !TT->isIncompleteType(); } @@ -333,7 +235,6 @@ static llvm::Type *getTypeForFormat(llvm::LLVMContext &VMContext, llvm::Type *CodeGenTypes::ConvertFunctionTypeInternal(QualType QFT) { assert(QFT.isCanonical()); - const Type *Ty = QFT.getTypePtr(); const FunctionType *FT = cast(QFT.getTypePtr()); // First, check whether we can build the full function type. If the // function type depends on an incomplete type (e.g. a struct or enum), we @@ -356,14 +257,6 @@ llvm::Type *CodeGenTypes::ConvertFunctionTypeInternal(QualType QFT) { return llvm::StructType::get(getLLVMContext()); } - // While we're converting the parameter types for a function, we don't want - // to recursively convert any pointed-to structs. Converting directly-used - // structs is ok though. - if (!RecordsBeingLaidOut.insert(Ty).second) { - SkippedLayout = true; - return llvm::StructType::get(getLLVMContext()); - } - // The function type can be built; call the appropriate routines to // build it. const CGFunctionInfo *FI; @@ -389,11 +282,6 @@ llvm::Type *CodeGenTypes::ConvertFunctionTypeInternal(QualType QFT) { ResultType = GetFunctionType(*FI); } - RecordsBeingLaidOut.erase(Ty); - - if (RecordsBeingLaidOut.empty()) - while (!DeferredRecords.empty()) - ConvertRecordDeclType(DeferredRecords.pop_back_val()); return ResultType; } @@ -421,27 +309,16 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) { if (const RecordType *RT = dyn_cast(Ty)) return ConvertRecordDeclType(RT->getDecl()); - // The LLVM type we return for a given Clang type may not always be the same, - // most notably when dealing with recursive structs. We mark these potential - // cases with ShouldUseCache below. Builtin types cannot be recursive. - // TODO: when clang uses LLVM opaque pointers we won't be able to represent - // recursive types with LLVM types, making this logic much simpler. llvm::Type *CachedType = nullptr; - bool ShouldUseCache = - Ty->isBuiltinType() || - (noRecordsBeingLaidOut() && FunctionsBeingProcessed.empty()); - if (ShouldUseCache) { - llvm::DenseMap::iterator TCI = - TypeCache.find(Ty); - if (TCI != TypeCache.end()) - CachedType = TCI->second; - // With expensive checks, check that the type we compute matches the - // cached type. + auto TCI = TypeCache.find(Ty); + if (TCI != TypeCache.end()) + CachedType = TCI->second; + // With expensive checks, check that the type we compute matches the + // cached type. #ifndef EXPENSIVE_CHECKS - if (CachedType) - return CachedType; + if (CachedType) + return CachedType; #endif - } // If we don't have it in the cache, convert it now. llvm::Type *ResultType = nullptr; @@ -835,8 +712,7 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) { assert((!CachedType || CachedType == ResultType) && "Cached type doesn't match computed type"); - if (ShouldUseCache) - TypeCache[Ty] = ResultType; + TypeCache[Ty] = ResultType; return ResultType; } @@ -869,17 +745,6 @@ llvm::StructType *CodeGenTypes::ConvertRecordDeclType(const RecordDecl *RD) { if (!RD || !RD->isCompleteDefinition() || !Ty->isOpaque()) return Ty; - // If converting this type would cause us to infinitely loop, don't do it! - if (!isSafeToConvert(RD, *this)) { - DeferredRecords.push_back(RD); - return Ty; - } - - // Okay, this is a definition of a type. Compile the implementation now. - bool InsertResult = RecordsBeingLaidOut.insert(Key).second; - (void)InsertResult; - assert(InsertResult && "Recursively compiling a struct?"); - // Force conversion of non-virtual base classes recursively. if (const CXXRecordDecl *CRD = dyn_cast(RD)) { for (const auto &I : CRD->bases()) { @@ -892,22 +757,12 @@ llvm::StructType *CodeGenTypes::ConvertRecordDeclType(const RecordDecl *RD) { std::unique_ptr Layout = ComputeRecordLayout(RD, Ty); CGRecordLayouts[Key] = std::move(Layout); - // We're done laying out this struct. - bool EraseResult = RecordsBeingLaidOut.erase(Key); (void)EraseResult; - assert(EraseResult && "struct not in RecordsBeingLaidOut set?"); - // If this struct blocked a FunctionType conversion, then recompute whatever // was derived from that. // FIXME: This is hugely overconservative. if (SkippedLayout) TypeCache.clear(); - // If we're done converting the outer-most record, then convert any deferred - // structs as well. - if (RecordsBeingLaidOut.empty()) - while (!DeferredRecords.empty()) - ConvertRecordDeclType(DeferredRecords.pop_back_val()); - return Ty; } diff --git a/clang/lib/CodeGen/CodeGenTypes.h b/clang/lib/CodeGen/CodeGenTypes.h index e76fda95513f6..9088f77b95c34 100644 --- a/clang/lib/CodeGen/CodeGenTypes.h +++ b/clang/lib/CodeGen/CodeGenTypes.h @@ -78,20 +78,12 @@ class CodeGenTypes { /// Hold memoized CGFunctionInfo results. llvm::FoldingSet FunctionInfos{FunctionInfosLog2InitSize}; - /// This set keeps track of records that we're currently converting - /// to an IR type. For example, when converting: - /// struct A { struct B { int x; } } when processing 'x', the 'A' and 'B' - /// types will be in this set. - llvm::SmallPtrSet RecordsBeingLaidOut; - llvm::SmallPtrSet FunctionsBeingProcessed; /// True if we didn't layout a function due to a being inside /// a recursive struct conversion, set this to true. bool SkippedLayout; - SmallVector DeferredRecords; - /// This map keeps cache of llvm::Types and maps clang::Type to /// corresponding llvm::Type. llvm::DenseMap TypeCache; @@ -300,12 +292,6 @@ class CodeGenTypes { bool isZeroInitializable(const RecordDecl *RD); bool isRecordLayoutComplete(const Type *Ty) const; - bool noRecordsBeingLaidOut() const { - return RecordsBeingLaidOut.empty(); - } - bool isRecordBeingLaidOut(const Type *Ty) const { - return RecordsBeingLaidOut.count(Ty); - } unsigned getTargetAddressSpace(QualType T) const; };