https://github.com/urnathan created 
https://github.com/llvm/llvm-project/pull/73264

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]

>From e4c92cd687782743ba939becf7ea8862eea6a108 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nat...@acm.org>
Date: Thu, 23 Nov 2023 11:30:12 -0500
Subject: [PATCH] [clang] Avoid recalculating TBAA base type info

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.
---
 clang/lib/CodeGen/CodeGenTBAA.cpp | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

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) {

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to