[clang] [clang] Avoid recalculating TBAA base type info (PR #73264)

[Nathan Sidwell via cfe-commits]

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

>From e4c92cd687782743ba939becf7ea8862eea6a108 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan at acm.org>
Date: Thu, 23 Nov 2023 11:30:12 -0500
Subject: [PATCH 1/2] [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) {

>From fee3b7407ab6fe73476eed82e387cceb9b6ef88c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell <nathan at acm.org>
Date: Fri, 1 Dec 2023 15:58:51 -0500
Subject: [PATCH 2/2] Use insert not try_emplace

---
 clang/lib/CodeGen/CodeGenTBAA.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 94dc304bfc243b9..5906b14dd93cf08 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -428,7 +428,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
   // the helper can recursively call us.
   llvm::MDNode *TypeNode = getBaseTypeInfoHelper(Ty);
   LLVM_ATTRIBUTE_UNUSED auto inserted =
-      BaseTypeMetadataCache.try_emplace(Ty, TypeNode);
+      BaseTypeMetadataCache.insert({Ty, TypeNode});
   assert(inserted.second && "BaseType metadata was already inserted");
 
   return TypeNode;