llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Nathan Sidwell (urnathan) <details> <summary>Changes</summary> I noticed that TBAA BaseTypeMetadataCache can legitimately store null values, but it also uses that to mean 'no entry'. Thus nullptr entries get continually recalculated. (AFAICT null entries can never become non-null.) This changes the hash lookup/insertion to use find and try_emplace rather than the subscript operator. While there, getBaseTypeInfoHelper will insert non-null values into the hashtable itself, but simply return nullptr values. The only caller, getBaseTypeInfo unconditionally inserts the value anyway. It seems clearer to let getBaseTypeInfo do the insertion and getBaseTypeInfoHelper merely computes. [This is the second of a pair of prs] --- Full diff: https://github.com/llvm/llvm-project/pull/73264.diff 1 Files Affected: - (modified) clang/lib/CodeGen/CodeGenTBAA.cpp (+15-9) ``````````diff diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp index 8705d3d65f1a573..94dc304bfc243b9 100644 --- a/clang/lib/CodeGen/CodeGenTBAA.cpp +++ b/clang/lib/CodeGen/CodeGenTBAA.cpp @@ -342,7 +342,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { // field. Virtual bases are more complex and omitted, but avoid an // incomplete view for NewStructPathTBAA. if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0) - return BaseTypeMetadataCache[Ty] = nullptr; + return nullptr; for (const CXXBaseSpecifier &B : CXXRD->bases()) { if (B.isVirtual()) continue; @@ -354,7 +354,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { ? getBaseTypeInfo(BaseQTy) : getTypeInfo(BaseQTy); if (!TypeNode) - return BaseTypeMetadataCache[Ty] = nullptr; + return nullptr; uint64_t Offset = Layout.getBaseClassOffset(BaseRD).getQuantity(); uint64_t Size = Context.getASTRecordLayout(BaseRD).getDataSize().getQuantity(); @@ -378,7 +378,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) { llvm::MDNode *TypeNode = isValidBaseType(FieldQTy) ? getBaseTypeInfo(FieldQTy) : getTypeInfo(FieldQTy); if (!TypeNode) - return BaseTypeMetadataCache[Ty] = nullptr; + return nullptr; uint64_t BitOffset = Layout.getFieldOffset(Field->getFieldIndex()); uint64_t Offset = Context.toCharUnitsFromBits(BitOffset).getQuantity(); @@ -418,14 +418,20 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) { return nullptr; const Type *Ty = Context.getCanonicalType(QTy).getTypePtr(); - if (llvm::MDNode *N = BaseTypeMetadataCache[Ty]) - return N; - // Note that the following helper call is allowed to add new nodes to the - // cache, which invalidates all its previously obtained iterators. So we - // first generate the node for the type and then add that node to the cache. + // nullptr is a valid value in the cache, so use find rather than [] + auto I = BaseTypeMetadataCache.find(Ty); + if (I != BaseTypeMetadataCache.end()) + return I->second; + + // First calculate the metadata, before recomputing the insertion point, as + // the helper can recursively call us. llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty); - return BaseTypeMetadataCache[Ty] = TypeNode; + LLVM_ATTRIBUTE_UNUSED auto inserted = + BaseTypeMetadataCache.try_emplace(Ty, TypeNode); + assert(inserted.second && "BaseType metadata was already inserted"); + + return TypeNode; } llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) { `````````` </details> https://github.com/llvm/llvm-project/pull/73264 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
