[clang] 3fe89be - [clang][RISCV][NFC] Prevent data race in RVVType::computeType

Kito Cheng via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 23 00:59:27 PST 2022


Author: Kito Cheng
Date: 2022-11-23T16:59:19+08:00
New Revision: 3fe89be8015955f2e8403f8b7d7580db13cedb2c

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

LOG: [clang][RISCV][NFC] Prevent data race in RVVType::computeType

Introduce a RVVTypeCache to hold the cache instead of using a local
static variable to maintain a cache.

Also made construct of RVVType to private, make sure that could be only
created by a cache manager.

Reviewed By: sammccall

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

Added: 
    

Modified: 
    clang/include/clang/Support/RISCVVIntrinsicUtils.h
    clang/lib/Sema/SemaRISCVVectorLookup.cpp
    clang/lib/Support/RISCVVIntrinsicUtils.cpp
    clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
index 4a8bbc78cdee3..8b9f994a4e31a 100644
--- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -15,7 +15,9 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include <cstdint>
+#include <set>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 namespace llvm {
@@ -182,9 +184,12 @@ struct LMULType {
 class RVVType;
 using RVVTypePtr = RVVType *;
 using RVVTypes = std::vector<RVVTypePtr>;
+class RVVTypeCache;
 
 // This class is compact representation of a valid and invalid RVVType.
 class RVVType {
+  friend class RVVTypeCache;
+
   BasicType BT;
   ScalarTypeKind ScalarType = Invalid;
   LMULType LMUL;
@@ -204,10 +209,9 @@ class RVVType {
 
   enum class FixedLMULType { LargerThan, SmallerThan };
 
-public:
-  RVVType() : BT(BasicType::Unknown), LMUL(0), Valid(false) {}
   RVVType(BasicType BT, int Log2LMUL, const PrototypeDescriptor &Profile);
 
+public:
   // Return the string representation of a type, which is an encoded string for
   // passing to the BUILTIN() macro in Builtins.def.
   const std::string &getBuiltinStr() const { return BuiltinStr; }
@@ -275,17 +279,25 @@ class RVVType {
   void initTypeStr();
   // Compute and record a short name of a type for C/C++ name suffix.
   void initShortStr();
+};
+
+// This class is used to manage RVVType, RVVType should only created by this
+// class, also provided thread-safe cache capability.
+class RVVTypeCache {
+private:
+  std::unordered_map<uint64_t, RVVType> LegalTypes;
+  std::set<uint64_t> IllegalTypes;
 
 public:
   /// Compute output and input types by applying 
diff erent config (basic type
   /// and LMUL with type transformers). It also record result of type in legal
   /// or illegal set to avoid compute the same config again. The result maybe
   /// have illegal RVVType.
-  static llvm::Optional<RVVTypes>
+  llvm::Optional<RVVTypes>
   computeTypes(BasicType BT, int Log2LMUL, unsigned NF,
                llvm::ArrayRef<PrototypeDescriptor> Prototype);
-  static llvm::Optional<RVVTypePtr> computeType(BasicType BT, int Log2LMUL,
-                                                PrototypeDescriptor Proto);
+  llvm::Optional<RVVTypePtr> computeType(BasicType BT, int Log2LMUL,
+                                         PrototypeDescriptor Proto);
 };
 
 enum PolicyScheme : uint8_t {
@@ -373,7 +385,7 @@ class RVVIntrinsic {
   std::string getBuiltinTypeStr() const;
 
   static std::string
-  getSuffixStr(BasicType Type, int Log2LMUL,
+  getSuffixStr(RVVTypeCache &TypeCache, BasicType Type, int Log2LMUL,
                llvm::ArrayRef<PrototypeDescriptor> PrototypeDescriptors);
 
   static llvm::SmallVector<PrototypeDescriptor>

diff  --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
index 4ab393b89d1b9..85bf11e942b96 100644
--- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -132,6 +132,7 @@ class RISCVIntrinsicManagerImpl : public sema::RISCVIntrinsicManager {
 private:
   Sema &S;
   ASTContext &Context;
+  RVVTypeCache TypeCache;
 
   // List of all RVV intrinsic.
   std::vector<RVVIntrinsicDef> IntrinsicList;
@@ -247,16 +248,16 @@ void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
           continue;
 
         Optional<RVVTypes> Types =
-            RVVType::computeTypes(BaseType, Log2LMUL, Record.NF, ProtoSeq);
+            TypeCache.computeTypes(BaseType, Log2LMUL, Record.NF, ProtoSeq);
 
         // Ignored to create new intrinsic if there are any illegal types.
         if (!Types.has_value())
           continue;
 
-        std::string SuffixStr =
-            RVVIntrinsic::getSuffixStr(BaseType, Log2LMUL, SuffixProto);
+        std::string SuffixStr = RVVIntrinsic::getSuffixStr(
+            TypeCache, BaseType, Log2LMUL, SuffixProto);
         std::string OverloadedSuffixStr = RVVIntrinsic::getSuffixStr(
-            BaseType, Log2LMUL, OverloadedSuffixProto);
+            TypeCache, BaseType, Log2LMUL, OverloadedSuffixProto);
 
         // Create non-masked intrinsic.
         InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr, false, *Types,
@@ -271,7 +272,7 @@ void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
                     BasicProtoSeq, /*IsMasked=*/false,
                     /*HasMaskedOffOperand=*/false, Record.HasVL, Record.NF,
                     Record.IsPrototypeDefaultTU, UnMaskedPolicyScheme, P);
-            Optional<RVVTypes> PolicyTypes = RVVType::computeTypes(
+            Optional<RVVTypes> PolicyTypes = TypeCache.computeTypes(
                 BaseType, Log2LMUL, Record.NF, PolicyPrototype);
             InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr,
                              /*IsMask=*/false, *PolicyTypes, UnMaskedHasPolicy,
@@ -282,7 +283,7 @@ void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
           continue;
         // Create masked intrinsic.
         Optional<RVVTypes> MaskTypes =
-            RVVType::computeTypes(BaseType, Log2LMUL, Record.NF, ProtoMaskSeq);
+            TypeCache.computeTypes(BaseType, Log2LMUL, Record.NF, ProtoMaskSeq);
         InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr, true,
                          *MaskTypes, MaskedHasPolicy, Policy::PolicyNone,
                          Record.IsPrototypeDefaultTU);
@@ -295,7 +296,7 @@ void RISCVIntrinsicManagerImpl::InitIntrinsicList() {
                   BasicProtoSeq, /*IsMasked=*/true, Record.HasMaskedOffOperand,
                   Record.HasVL, Record.NF, Record.IsPrototypeDefaultTU,
                   MaskedPolicyScheme, P);
-          Optional<RVVTypes> PolicyTypes = RVVType::computeTypes(
+          Optional<RVVTypes> PolicyTypes = TypeCache.computeTypes(
               BaseType, Log2LMUL, Record.NF, PolicyPrototype);
           InitRVVIntrinsic(Record, SuffixStr, OverloadedSuffixStr,
                            /*IsMask=*/true, *PolicyTypes, MaskedHasPolicy, P,

diff  --git a/clang/lib/Support/RISCVVIntrinsicUtils.cpp b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
index 87a89feb4b0d8..92e7fcbb91243 100644
--- a/clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -16,8 +16,6 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
 #include <numeric>
-#include <set>
-#include <unordered_map>
 
 using namespace llvm;
 
@@ -786,8 +784,8 @@ void RVVType::applyFixedLog2LMUL(int Log2LMUL, enum FixedLMULType Type) {
 }
 
 Optional<RVVTypes>
-RVVType::computeTypes(BasicType BT, int Log2LMUL, unsigned NF,
-                      ArrayRef<PrototypeDescriptor> Prototype) {
+RVVTypeCache::computeTypes(BasicType BT, int Log2LMUL, unsigned NF,
+                           ArrayRef<PrototypeDescriptor> Prototype) {
   // LMUL x NF must be less than or equal to 8.
   if ((Log2LMUL >= 1) && (1 << Log2LMUL) * NF > 8)
     return llvm::None;
@@ -816,11 +814,8 @@ static uint64_t computeRVVTypeHashValue(BasicType BT, int Log2LMUL,
          ((uint64_t)(Proto.VTM & 0xff) << 32);
 }
 
-Optional<RVVTypePtr> RVVType::computeType(BasicType BT, int Log2LMUL,
-                                          PrototypeDescriptor Proto) {
-  // Concat BasicType, LMUL and Proto as key
-  static std::unordered_map<uint64_t, RVVType> LegalTypes;
-  static std::set<uint64_t> IllegalTypes;
+Optional<RVVTypePtr> RVVTypeCache::computeType(BasicType BT, int Log2LMUL,
+                                               PrototypeDescriptor Proto) {
   uint64_t Idx = computeRVVTypeHashValue(BT, Log2LMUL, Proto);
   // Search first
   auto It = LegalTypes.find(Idx);
@@ -834,8 +829,9 @@ Optional<RVVTypePtr> RVVType::computeType(BasicType BT, int Log2LMUL,
   RVVType T(BT, Log2LMUL, Proto);
   if (T.isValid()) {
     // Record legal type index and value.
-    LegalTypes.insert({Idx, T});
-    return &(LegalTypes[Idx]);
+    std::pair<std::unordered_map<uint64_t, RVVType>::iterator, bool>
+        InsertResult = LegalTypes.insert({Idx, T});
+    return &(InsertResult.first->second);
   }
   // Record illegal type index.
   IllegalTypes.insert(Idx);
@@ -900,11 +896,11 @@ std::string RVVIntrinsic::getBuiltinTypeStr() const {
 }
 
 std::string RVVIntrinsic::getSuffixStr(
-    BasicType Type, int Log2LMUL,
+    RVVTypeCache &TypeCache, BasicType Type, int Log2LMUL,
     llvm::ArrayRef<PrototypeDescriptor> PrototypeDescriptors) {
   SmallVector<std::string> SuffixStrs;
   for (auto PD : PrototypeDescriptors) {
-    auto T = RVVType::computeType(Type, Log2LMUL, PD);
+    auto T = TypeCache.computeType(Type, Log2LMUL, PD);
     SuffixStrs.push_back((*T)->getShortStr());
   }
   return join(SuffixStrs, "_");

diff  --git a/clang/utils/TableGen/RISCVVEmitter.cpp b/clang/utils/TableGen/RISCVVEmitter.cpp
index 84f0420d7c331..253e9a79dd337 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -95,6 +95,7 @@ class SemaSignatureTable {
 class RVVEmitter {
 private:
   RecordKeeper &Records;
+  RVVTypeCache TypeCache;
 
 public:
   RVVEmitter(RecordKeeper &R) : Records(R) {}
@@ -349,8 +350,8 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
   constexpr int Log2LMULs[] = {-3, -2, -1, 0, 1, 2, 3};
   // Print RVV boolean types.
   for (int Log2LMUL : Log2LMULs) {
-    auto T = RVVType::computeType(BasicType::Int8, Log2LMUL,
-                                  PrototypeDescriptor::Mask);
+    auto T = TypeCache.computeType(BasicType::Int8, Log2LMUL,
+                                   PrototypeDescriptor::Mask);
     if (T)
       printType(T.value());
   }
@@ -358,10 +359,10 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
   for (char I : StringRef("csil")) {
     BasicType BT = ParseBasicType(I);
     for (int Log2LMUL : Log2LMULs) {
-      auto T = RVVType::computeType(BT, Log2LMUL, PrototypeDescriptor::Vector);
+      auto T = TypeCache.computeType(BT, Log2LMUL, PrototypeDescriptor::Vector);
       if (T) {
         printType(T.value());
-        auto UT = RVVType::computeType(
+        auto UT = TypeCache.computeType(
             BT, Log2LMUL,
             PrototypeDescriptor(BaseTypeModifier::Vector,
                                 VectorTypeModifier::NoModifier,
@@ -372,8 +373,8 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
   }
   OS << "#if defined(__riscv_zvfh)\n";
   for (int Log2LMUL : Log2LMULs) {
-    auto T = RVVType::computeType(BasicType::Float16, Log2LMUL,
-                                  PrototypeDescriptor::Vector);
+    auto T = TypeCache.computeType(BasicType::Float16, Log2LMUL,
+                                   PrototypeDescriptor::Vector);
     if (T)
       printType(T.value());
   }
@@ -381,8 +382,8 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
 
   OS << "#if (__riscv_v_elen_fp >= 32)\n";
   for (int Log2LMUL : Log2LMULs) {
-    auto T = RVVType::computeType(BasicType::Float32, Log2LMUL,
-                                  PrototypeDescriptor::Vector);
+    auto T = TypeCache.computeType(BasicType::Float32, Log2LMUL,
+                                   PrototypeDescriptor::Vector);
     if (T)
       printType(T.value());
   }
@@ -390,8 +391,8 @@ void RVVEmitter::createHeader(raw_ostream &OS) {
 
   OS << "#if (__riscv_v_elen_fp >= 64)\n";
   for (int Log2LMUL : Log2LMULs) {
-    auto T = RVVType::computeType(BasicType::Float64, Log2LMUL,
-                                  PrototypeDescriptor::Vector);
+    auto T = TypeCache.computeType(BasicType::Float64, Log2LMUL,
+                                   PrototypeDescriptor::Vector);
     if (T)
       printType(T.value());
   }
@@ -553,14 +554,15 @@ void RVVEmitter::createRVVIntrinsics(
       for (int Log2LMUL : Log2LMULList) {
         BasicType BT = ParseBasicType(I);
         Optional<RVVTypes> Types =
-            RVVType::computeTypes(BT, Log2LMUL, NF, Prototype);
+            TypeCache.computeTypes(BT, Log2LMUL, NF, Prototype);
         // Ignored to create new intrinsic if there are any illegal types.
         if (!Types)
           continue;
 
-        auto SuffixStr = RVVIntrinsic::getSuffixStr(BT, Log2LMUL, SuffixDesc);
-        auto OverloadedSuffixStr =
-            RVVIntrinsic::getSuffixStr(BT, Log2LMUL, OverloadedSuffixDesc);
+        auto SuffixStr =
+            RVVIntrinsic::getSuffixStr(TypeCache, BT, Log2LMUL, SuffixDesc);
+        auto OverloadedSuffixStr = RVVIntrinsic::getSuffixStr(
+            TypeCache, BT, Log2LMUL, OverloadedSuffixDesc);
         // Create a unmasked intrinsic
         Out.push_back(std::make_unique<RVVIntrinsic>(
             Name, SuffixStr, OverloadedName, OverloadedSuffixStr, IRName,
@@ -576,7 +578,7 @@ void RVVEmitter::createRVVIntrinsics(
                     /*HasMaskedOffOperand=*/false, HasVL, NF,
                     IsPrototypeDefaultTU, UnMaskedPolicyScheme, P);
             Optional<RVVTypes> PolicyTypes =
-                RVVType::computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
+                TypeCache.computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
             Out.push_back(std::make_unique<RVVIntrinsic>(
                 Name, SuffixStr, OverloadedName, OverloadedSuffixStr, IRName,
                 /*IsMask=*/false, /*HasMaskedOffOperand=*/false, HasVL,
@@ -588,7 +590,7 @@ void RVVEmitter::createRVVIntrinsics(
           continue;
         // Create a masked intrinsic
         Optional<RVVTypes> MaskTypes =
-            RVVType::computeTypes(BT, Log2LMUL, NF, Prototype);
+            TypeCache.computeTypes(BT, Log2LMUL, NF, Prototype);
         Out.push_back(std::make_unique<RVVIntrinsic>(
             Name, SuffixStr, OverloadedName, OverloadedSuffixStr, MaskedIRName,
             /*IsMasked=*/true, HasMaskedOffOperand, HasVL, MaskedPolicyScheme,
@@ -603,7 +605,7 @@ void RVVEmitter::createRVVIntrinsics(
                   BasicPrototype, /*IsMasked=*/true, HasMaskedOffOperand, HasVL,
                   NF, IsPrototypeDefaultTU, MaskedPolicyScheme, P);
           Optional<RVVTypes> PolicyTypes =
-              RVVType::computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
+              TypeCache.computeTypes(BT, Log2LMUL, NF, PolicyPrototype);
           Out.push_back(std::make_unique<RVVIntrinsic>(
               Name, SuffixStr, OverloadedName, OverloadedSuffixStr,
               MaskedIRName, /*IsMasked=*/true, HasMaskedOffOperand, HasVL,


        


More information about the cfe-commits mailing list