[clang] f05a63f - [clang] Properly cache member pointer LLVM types

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-02-08T13:22:24-08:00
New Revision: f05a63f9a09bdacab6a98683c5a4fc3f1f2a9149

URL: https://github.com/llvm/llvm-project/commit/f05a63f9a09bdacab6a98683c5a4fc3f1f2a9149
DIFF: https://github.com/llvm/llvm-project/commit/f05a63f9a09bdacab6a98683c5a4fc3f1f2a9149.diff

LOG: [clang] Properly cache member pointer LLVM types

When not going through the main Clang->LLVM type cache, we'd
accidentally create multiple different opaque types for a member pointer
type.

This allows us to remove the -verify-type-cache flag now that
check-clang passes with it on. We can do the verification in expensive
builds. Previously microsoft-abi-member-pointers.cpp was failing with
-verify-type-cache.

I suspect that there may be more issues when we have multiple member
pointer types and we clear the cache, but we can leave that for later.

Followup to D118744.

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D119215

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenTypes.cpp
    clang/lib/CodeGen/CodeGenTypes.h
    clang/test/CodeGenCXX/type-cache-2.cpp
    clang/test/CodeGenCXX/type-cache-3.cpp
    clang/test/CodeGenCXX/type-cache.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index 7a8a7c916473..33bdb6911cd1 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -29,16 +29,6 @@
 using namespace clang;
 using namespace CodeGen;
 
-#ifndef NDEBUG
-#include "llvm/Support/CommandLine.h"
-// TODO: turn on by default when defined(EXPENSIVE_CHECKS) once check-clang is
-// -verify-type-cache clean.
-static llvm::cl::opt<bool> VerifyTypeCache(
-    "verify-type-cache",
-    llvm::cl::desc("Verify that the type cache matches the computed type"),
-    llvm::cl::init(false), llvm::cl::Hidden);
-#endif
-
 CodeGenTypes::CodeGenTypes(CodeGenModule &cgm)
   : CGM(cgm), Context(cgm.getContext()), TheModule(cgm.getModule()),
     Target(cgm.getTarget()), TheCXXABI(cgm.getCXXABI()),
@@ -437,14 +427,12 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
         TypeCache.find(Ty);
     if (TCI != TypeCache.end())
       CachedType = TCI->second;
-    if (CachedType) {
-#ifndef NDEBUG
-      if (!VerifyTypeCache)
-        return CachedType;
-#else
+      // With expensive checks, check that the type we compute matches the
+      // cached type.
+#ifndef EXPENSIVE_CHECKS
+    if (CachedType)
       return CachedType;
 #endif
-    }
   }
 
   // If we don't have it in the cache, convert it now.
@@ -784,8 +772,11 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
   case Type::MemberPointer: {
     auto *MPTy = cast<MemberPointerType>(Ty);
     if (!getCXXABI().isMemberPointerConvertible(MPTy)) {
-      RecordsWithOpaqueMemberPointers.insert(MPTy->getClass());
-      ResultType = llvm::StructType::create(getLLVMContext());
+      auto *C = MPTy->getClass();
+      auto Insertion = RecordsWithOpaqueMemberPointers.insert({C, nullptr});
+      if (Insertion.second)
+        Insertion.first->second = llvm::StructType::create(getLLVMContext());
+      ResultType = Insertion.first->second;
     } else {
       ResultType = getCXXABI().ConvertMemberPointerType(MPTy);
     }
@@ -822,13 +813,8 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
   }
 
   assert(ResultType && "Didn't convert a type?");
-
-#ifndef NDEBUG
-  if (CachedType) {
-    assert(CachedType == ResultType &&
-           "Cached type doesn't match computed type");
-  }
-#endif
+  assert((!CachedType || CachedType == ResultType) &&
+         "Cached type doesn't match computed type");
 
   if (ShouldUseCache)
     TypeCache[Ty] = ResultType;

diff  --git a/clang/lib/CodeGen/CodeGenTypes.h b/clang/lib/CodeGen/CodeGenTypes.h
index 05aae88ba59e..cd20563cbf75 100644
--- a/clang/lib/CodeGen/CodeGenTypes.h
+++ b/clang/lib/CodeGen/CodeGenTypes.h
@@ -96,7 +96,7 @@ class CodeGenTypes {
   /// corresponding llvm::Type.
   llvm::DenseMap<const Type *, llvm::Type *> TypeCache;
 
-  llvm::SmallSet<const Type *, 8> RecordsWithOpaqueMemberPointers;
+  llvm::DenseMap<const Type *, llvm::Type *> RecordsWithOpaqueMemberPointers;
 
   static constexpr unsigned FunctionInfosLog2InitSize = 9;
   /// Helper for ConvertType.

diff  --git a/clang/test/CodeGenCXX/type-cache-2.cpp b/clang/test/CodeGenCXX/type-cache-2.cpp
index bc672f795166..1f1472a0ebac 100644
--- a/clang/test/CodeGenCXX/type-cache-2.cpp
+++ b/clang/test/CodeGenCXX/type-cache-2.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -verify-type-cache -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
 // REQUIRES: asserts, x86-registered-target
 
 // CHECK: call void @"?dc at z@@SAXU1@@Z"

diff  --git a/clang/test/CodeGenCXX/type-cache-3.cpp b/clang/test/CodeGenCXX/type-cache-3.cpp
index 2cbb63b143de..8a9af353f58e 100644
--- a/clang/test/CodeGenCXX/type-cache-3.cpp
+++ b/clang/test/CodeGenCXX/type-cache-3.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -verify-type-cache -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
 // REQUIRES: asserts, x86-registered-target
 
 // CHECK-LABEL: define {{.*}}@"?f@@YAXXZ"(

diff  --git a/clang/test/CodeGenCXX/type-cache.cpp b/clang/test/CodeGenCXX/type-cache.cpp
index 02caee8e0f62..2734c77762d8 100644
--- a/clang/test/CodeGenCXX/type-cache.cpp
+++ b/clang/test/CodeGenCXX/type-cache.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -mllvm -verify-type-cache -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple i386-pc-windows-msvc19.16.0 | FileCheck %s
 // REQUIRES: asserts, x86-registered-target
 
 // CHECK: call {}* @"?f@@YA?AUz@@XZ"()