llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Fangrui Song (MaskRay) <details> <summary>Changes</summary> Eight of the ASTContext type pools key on a QualType, or on a QualType and a bool. The `get*Type()` functions incur the FoldingSetNodeID serialization overhead before probing the hash table. Switch to UniquingSet. Define `QualTypeBoolInfo` for three pools that key on a QualType and a bool, because DenseMapInfo has no bool specialization. After the canonical type is built, these getters re-probe to refresh the insert token. A token is now a hash (#<!-- -->218190), not a bucket, so nothing invalidates it; fold the re-probe into the assertion it feeds. Aided by Opus 5 --- Full diff: https://github.com/llvm/llvm-project/pull/221850.diff 3 Files Affected: - (modified) clang/include/clang/AST/ASTContext.h (+26-8) - (modified) clang/include/clang/AST/TypeBase.h (+12-55) - (modified) clang/lib/AST/ASTContext.cpp (+21-57) ``````````diff diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index f875ae365b892..8277f1fcced95 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -218,6 +218,22 @@ struct PFPField { FieldDecl *Field; }; +/// UniquingSet info for pools keyed on a QualType and a bool. DenseMapInfo has +/// no bool specialization, so we cannot use the default UniquingSetInfo. +struct QualTypeBoolInfo { + using KeyTy = std::pair<QualType, bool>; + + template <typename T> static KeyTy getKey(const T &N) { return N.getKey(); } + + static unsigned getHashValue(const KeyTy &Key) { + return llvm::hash_combine(Key.first.getAsOpaquePtr(), Key.second); + } + + template <typename T> static bool isEqual(const KeyTy &Key, const T &N) { + return Key == N.getKey(); + } +}; + /// Holds long-lived AST nodes (such as types and decls) that can be /// referred to throughout the semantic analysis of a file. class ASTContext : public RefCountedBase<ASTContext> { @@ -225,12 +241,14 @@ class ASTContext : public RefCountedBase<ASTContext> { mutable SmallVector<Type *, 0> Types; mutable llvm::FoldingSet<ExtQuals> ExtQualNodes; - mutable llvm::FoldingSet<ComplexType> ComplexTypes; - mutable llvm::FoldingSet<PointerType> PointerTypes{GeneralTypesLog2InitSize}; + mutable llvm::UniquingSet<ComplexType> ComplexTypes; + mutable llvm::UniquingSet<PointerType> PointerTypes{GeneralTypesLog2InitSize}; mutable llvm::FoldingSet<AdjustedType> AdjustedTypes; - mutable llvm::FoldingSet<BlockPointerType> BlockPointerTypes; - mutable llvm::FoldingSet<LValueReferenceType> LValueReferenceTypes; - mutable llvm::FoldingSet<RValueReferenceType> RValueReferenceTypes; + mutable llvm::UniquingSet<BlockPointerType> BlockPointerTypes; + mutable llvm::UniquingSet<LValueReferenceType, QualTypeBoolInfo> + LValueReferenceTypes; + mutable llvm::UniquingSet<RValueReferenceType, QualTypeBoolInfo> + RValueReferenceTypes; mutable llvm::FoldingSet<MemberPointerType> MemberPointerTypes; mutable llvm::ContextualFoldingSet<ConstantArrayType, ASTContext &> ConstantArrayTypes; @@ -269,7 +287,7 @@ class ASTContext : public RefCountedBase<ASTContext> { SubstBuiltinTemplatePackTypes; mutable llvm::ContextualFoldingSet<TemplateSpecializationType, ASTContext&> TemplateSpecializationTypes; - mutable llvm::FoldingSet<ParenType> ParenTypes{GeneralTypesLog2InitSize}; + mutable llvm::UniquingSet<ParenType> ParenTypes{GeneralTypesLog2InitSize}; mutable llvm::FoldingSet<TagTypeFoldingSetPlaceholder> TagTypes; mutable llvm::FoldingSet<FoldingSetPlaceholder<UnresolvedUsingType>> UnresolvedUsingTypes; @@ -289,10 +307,10 @@ class ASTContext : public RefCountedBase<ASTContext> { mutable llvm::DenseMap<llvm::FoldingSetNodeIDRef, AutoType *> AutoTypes; mutable llvm::FoldingSet<DeducedTemplateSpecializationType> DeducedTemplateSpecializationTypes; - mutable llvm::FoldingSet<AtomicType> AtomicTypes; + mutable llvm::UniquingSet<AtomicType> AtomicTypes; mutable llvm::ContextualFoldingSet<AttributedType, ASTContext &> AttributedTypes; - mutable llvm::FoldingSet<PipeType> PipeTypes; + mutable llvm::UniquingSet<PipeType, QualTypeBoolInfo> PipeTypes; mutable llvm::FoldingSet<BitIntType> BitIntTypes; mutable llvm::ContextualFoldingSet<DependentBitIntType, ASTContext &> DependentBitIntTypes; diff --git a/clang/include/clang/AST/TypeBase.h b/clang/include/clang/AST/TypeBase.h index cdbd20b62bac5..9edf8a0ce9c68 100644 --- a/clang/include/clang/AST/TypeBase.h +++ b/clang/include/clang/AST/TypeBase.h @@ -3367,13 +3367,7 @@ class ComplexType : public Type, public llvm::FoldingSetNode { bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getElementType()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType Element) { - ID.AddPointer(Element.getAsOpaquePtr()); - } + QualType getKey() const { return getElementType(); } static bool classof(const Type *T) { return T->getTypeClass() == Complex; } }; @@ -3393,13 +3387,7 @@ class ParenType : public Type, public llvm::FoldingSetNode { bool isSugared() const { return true; } QualType desugar() const { return getInnerType(); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getInnerType()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType Inner) { - Inner.Profile(ID); - } + QualType getKey() const { return getInnerType(); } static bool classof(const Type *T) { return T->getTypeClass() == Paren; } }; @@ -3420,13 +3408,7 @@ class PointerType : public Type, public llvm::FoldingSetNode { bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getPointeeType()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType Pointee) { - ID.AddPointer(Pointee.getAsOpaquePtr()); - } + QualType getKey() const { return getPointeeType(); } static bool classof(const Type *T) { return T->getTypeClass() == Pointer; } }; @@ -3670,13 +3652,7 @@ class BlockPointerType : public Type, public llvm::FoldingSetNode { bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getPointeeType()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType Pointee) { - ID.AddPointer(Pointee.getAsOpaquePtr()); - } + QualType getKey() const { return getPointeeType(); } static bool classof(const Type *T) { return T->getTypeClass() == BlockPointer; @@ -3702,6 +3678,10 @@ class ReferenceType : public Type, public llvm::FoldingSetNode { QualType getPointeeTypeAsWritten() const { return PointeeType; } + std::pair<QualType, bool> getKey() const { + return {getPointeeTypeAsWritten(), isSpelledAsLValue()}; + } + QualType getPointeeType() const { // FIXME: this might strip inner qualifiers; okay? const ReferenceType *T = this; @@ -3710,17 +3690,6 @@ class ReferenceType : public Type, public llvm::FoldingSetNode { return T->PointeeType; } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, PointeeType, isSpelledAsLValue()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, - QualType Referencee, - bool SpelledAsLValue) { - ID.AddPointer(Referencee.getAsOpaquePtr()); - ID.AddBoolean(SpelledAsLValue); - } - static bool classof(const Type *T) { return T->getTypeClass() == LValueReference || T->getTypeClass() == RValueReference; @@ -8280,7 +8249,6 @@ class ObjCObjectPointerType : public Type, public llvm::FoldingSetNode { static void Profile(llvm::FoldingSetNodeID &ID, QualType T) { ID.AddPointer(T.getAsOpaquePtr()); } - static bool classof(const Type *T) { return T->getTypeClass() == ObjCObjectPointer; } @@ -8299,17 +8267,11 @@ class AtomicType : public Type, public llvm::FoldingSetNode { /// the type returned by performing an atomic load of this atomic type. QualType getValueType() const { return ValueType; } + QualType getKey() const { return getValueType(); } + bool isSugared() const { return false; } QualType desugar() const { return QualType(this, 0); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getValueType()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType T) { - ID.AddPointer(T.getAsOpaquePtr()); - } - static bool classof(const Type *T) { return T->getTypeClass() == Atomic; } @@ -8333,13 +8295,8 @@ class PipeType : public Type, public llvm::FoldingSetNode { QualType desugar() const { return QualType(this, 0); } - void Profile(llvm::FoldingSetNodeID &ID) { - Profile(ID, getElementType(), isReadOnly()); - } - - static void Profile(llvm::FoldingSetNodeID &ID, QualType T, bool isRead) { - ID.AddPointer(T.getAsOpaquePtr()); - ID.AddBoolean(isRead); + std::pair<QualType, bool> getKey() const { + return {getElementType(), isReadOnly()}; } static bool classof(const Type *T) { diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 84455fb6396bd..a8b256167fd64 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -3971,11 +3971,8 @@ void ASTContext::adjustExceptionSpec( QualType ASTContext::getComplexType(QualType T) const { // Unique pointers, to guarantee there is only one pointer of a particular // structure. - llvm::FoldingSetNodeID ID; - ComplexType::Profile(ID, T); - llvm::FoldingSetInsertToken Token; - if (ComplexType *CT = ComplexTypes.lookup(ID, Token)) + if (ComplexType *CT = ComplexTypes.lookup(T, Token)) return QualType(CT, 0); // If the pointee type isn't canonical, this won't be a canonical type either, @@ -3984,9 +3981,7 @@ QualType ASTContext::getComplexType(QualType T) const { if (!T.isCanonical()) { Canonical = getComplexType(getCanonicalType(T)); - // Get the new insert position for the node we care about. - ComplexType *NewIP = ComplexTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!ComplexTypes.lookup(T, Token) && "Shouldn't be in the map!"); } auto *New = new (*this, alignof(ComplexType)) ComplexType(T, Canonical); Types.push_back(New); @@ -3999,11 +3994,8 @@ QualType ASTContext::getComplexType(QualType T) const { QualType ASTContext::getPointerType(QualType T) const { // Unique pointers, to guarantee there is only one pointer of a particular // structure. - llvm::FoldingSetNodeID ID; - PointerType::Profile(ID, T); - llvm::FoldingSetInsertToken Token; - if (PointerType *PT = PointerTypes.lookup(ID, Token)) + if (PointerType *PT = PointerTypes.lookup(T, Token)) return QualType(PT, 0); // If the pointee type isn't canonical, this won't be a canonical type either, @@ -4012,9 +4004,7 @@ QualType ASTContext::getPointerType(QualType T) const { if (!T.isCanonical()) { Canonical = getPointerType(getCanonicalType(T)); - // Get the new insert position for the node we care about. - PointerType *NewIP = PointerTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!PointerTypes.lookup(T, Token) && "Shouldn't be in the map!"); } auto *New = new (*this, alignof(PointerType)) PointerType(T, Canonical); Types.push_back(New); @@ -4123,11 +4113,8 @@ QualType ASTContext::getBlockPointerType(QualType T) const { assert(T->isFunctionType() && "block of function types only"); // Unique pointers, to guarantee there is only one block of a particular // structure. - llvm::FoldingSetNodeID ID; - BlockPointerType::Profile(ID, T); - llvm::FoldingSetInsertToken Token; - if (BlockPointerType *PT = BlockPointerTypes.lookup(ID, Token)) + if (BlockPointerType *PT = BlockPointerTypes.lookup(T, Token)) return QualType(PT, 0); // If the block pointee type isn't canonical, this won't be a canonical @@ -4136,9 +4123,7 @@ QualType ASTContext::getBlockPointerType(QualType T) const { if (!T.isCanonical()) { Canonical = getBlockPointerType(getCanonicalType(T)); - // Get the new insert position for the node we care about. - BlockPointerType *NewIP = BlockPointerTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!BlockPointerTypes.lookup(T, Token) && "Shouldn't be in the map!"); } auto *New = new (*this, alignof(BlockPointerType)) BlockPointerType(T, Canonical); @@ -4157,11 +4142,9 @@ ASTContext::getLValueReferenceType(QualType T, bool SpelledAsLValue) const { // Unique pointers, to guarantee there is only one pointer of a particular // structure. - llvm::FoldingSetNodeID ID; - ReferenceType::Profile(ID, T, SpelledAsLValue); - llvm::FoldingSetInsertToken Token; - if (LValueReferenceType *RT = LValueReferenceTypes.lookup(ID, Token)) + if (LValueReferenceType *RT = + LValueReferenceTypes.lookup({T, SpelledAsLValue}, Token)) return QualType(RT, 0); const auto *InnerRef = T->getAs<ReferenceType>(); @@ -4173,9 +4156,8 @@ ASTContext::getLValueReferenceType(QualType T, bool SpelledAsLValue) const { QualType PointeeType = (InnerRef ? InnerRef->getPointeeType() : T); Canonical = getLValueReferenceType(getCanonicalType(PointeeType)); - // Get the new insert position for the node we care about. - LValueReferenceType *NewIP = LValueReferenceTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!LValueReferenceTypes.lookup({T, SpelledAsLValue}, Token) && + "Shouldn't be in the map!"); } auto *New = new (*this, alignof(LValueReferenceType)) @@ -4195,11 +4177,8 @@ QualType ASTContext::getRValueReferenceType(QualType T) const { // Unique pointers, to guarantee there is only one pointer of a particular // structure. - llvm::FoldingSetNodeID ID; - ReferenceType::Profile(ID, T, false); - llvm::FoldingSetInsertToken Token; - if (RValueReferenceType *RT = RValueReferenceTypes.lookup(ID, Token)) + if (RValueReferenceType *RT = RValueReferenceTypes.lookup({T, false}, Token)) return QualType(RT, 0); const auto *InnerRef = T->getAs<ReferenceType>(); @@ -4211,9 +4190,8 @@ QualType ASTContext::getRValueReferenceType(QualType T) const { QualType PointeeType = (InnerRef ? InnerRef->getPointeeType() : T); Canonical = getRValueReferenceType(getCanonicalType(PointeeType)); - // Get the new insert position for the node we care about. - RValueReferenceType *NewIP = RValueReferenceTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!RValueReferenceTypes.lookup({T, false}, Token) && + "Shouldn't be in the map!"); } auto *New = new (*this, alignof(RValueReferenceType)) @@ -5207,11 +5185,8 @@ QualType ASTContext::getFunctionTypeInternal( } QualType ASTContext::getPipeType(QualType T, bool ReadOnly) const { - llvm::FoldingSetNodeID ID; - PipeType::Profile(ID, T, ReadOnly); - llvm::FoldingSetInsertToken Token; - if (PipeType *PT = PipeTypes.lookup(ID, Token)) + if (PipeType *PT = PipeTypes.lookup({T, ReadOnly}, Token)) return QualType(PT, 0); // If the pipe element type isn't canonical, this won't be a canonical type @@ -5220,10 +5195,8 @@ QualType ASTContext::getPipeType(QualType T, bool ReadOnly) const { if (!T.isCanonical()) { Canonical = getPipeType(getCanonicalType(T), ReadOnly); - // Get the new insert position for the node we care about. - PipeType *NewIP = PipeTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); - (void)NewIP; + assert(!PipeTypes.lookup({T, ReadOnly}, Token) && + "Shouldn't be in the map!"); } auto *New = new (*this, alignof(PipeType)) PipeType(T, Canonical, ReadOnly); Types.push_back(New); @@ -6197,20 +6170,16 @@ QualType ASTContext::getTemplateSpecializationType( QualType ASTContext::getParenType(QualType InnerType) const { - llvm::FoldingSetNodeID ID; - ParenType::Profile(ID, InnerType); - llvm::FoldingSetInsertToken Token; - ParenType *T = ParenTypes.lookup(ID, Token); + ParenType *T = ParenTypes.lookup(InnerType, Token); if (T) return QualType(T, 0); QualType Canon = InnerType; if (!Canon.isCanonical()) { Canon = getCanonicalType(InnerType); - ParenType *CheckT = ParenTypes.lookup(ID, Token); - assert(!CheckT && "Paren canonical type broken"); - (void)CheckT; + assert(!ParenTypes.lookup(InnerType, Token) && + "Paren canonical type broken"); } T = new (*this, alignof(ParenType)) ParenType(InnerType, Canon); @@ -7003,11 +6972,8 @@ QualType ASTContext::getDeducedTemplateSpecializationType( QualType ASTContext::getAtomicType(QualType T) const { // Unique pointers, to guarantee there is only one pointer of a particular // structure. - llvm::FoldingSetNodeID ID; - AtomicType::Profile(ID, T); - llvm::FoldingSetInsertToken Token; - if (AtomicType *AT = AtomicTypes.lookup(ID, Token)) + if (AtomicType *AT = AtomicTypes.lookup(T, Token)) return QualType(AT, 0); // If the atomic value type isn't canonical, this won't be a canonical type @@ -7016,9 +6982,7 @@ QualType ASTContext::getAtomicType(QualType T) const { if (!T.isCanonical()) { Canonical = getAtomicType(getCanonicalType(T)); - // Get the new insert position for the node we care about. - AtomicType *NewIP = AtomicTypes.lookup(ID, Token); - assert(!NewIP && "Shouldn't be in the map!"); (void)NewIP; + assert(!AtomicTypes.lookup(T, Token) && "Shouldn't be in the map!"); } auto *New = new (*this, alignof(AtomicType)) AtomicType(T, Canonical); Types.push_back(New); `````````` </details> https://github.com/llvm/llvm-project/pull/221850 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
