[NFC] Optimize adding of decorations (#1233)

* [NFC] Optimize adding of decorations

Use std::undordered_set instead of std::multiset with custom comparator.
It is a bit unclear why std::multiset was required since there is no
order requirement for OpDecorate instructions in SPIR-V spec, in addition old
data structure was highly inefficient.

lib/SPIRV/libSPIRV/SPIRVDecorate.cpp

@@ -43,8 +43,8 @@
 #include "SPIRVValue.h"
 
 namespace SPIRV {
-template <class T, class B>
-spv_ostream &operator<<(spv_ostream &O, const std::multiset<T *, B> &V) {
+template <class T>
+spv_ostream &operator<<(spv_ostream &O, const std::vector<T *> &V) {
   for (auto &I : V)
     O << *I;
   return O;
@@ -244,57 +244,4 @@ void SPIRVGroupMemberDecorate::decorateTargets() {
     }
   }
 }
-
-bool SPIRVDecorateGeneric::Comparator::
-operator()(const SPIRVDecorateGeneric *A, const SPIRVDecorateGeneric *B) const {
-  auto Action = [=]() {
-    if (A->getOpCode() < B->getOpCode())
-      return true;
-    if (A->getOpCode() > B->getOpCode())
-      return false;
-    if (A->getDecorateKind() < B->getDecorateKind())
-      return true;
-    if (A->getDecorateKind() > B->getDecorateKind())
-      return false;
-    if (A->getLiteralCount() < B->getLiteralCount())
-      return true;
-    if (A->getLiteralCount() > B->getLiteralCount())
-      return false;
-    for (size_t I = 0, E = A->getLiteralCount(); I != E; ++I) {
-      auto EA = A->getLiteral(I);
-      auto EB = B->getLiteral(I);
-      if (EA < EB)
-        return true;
-      if (EA > EB)
-        return false;
-    }
-    return false;
-  };
-  auto Res = Action();
-  return Res;
-}
-
-bool operator==(const SPIRVDecorateGeneric &A, const SPIRVDecorateGeneric &B) {
-  if (A.getTargetId() != B.getTargetId())
-    return false;
-  if (A.getOpCode() != B.getOpCode())
-    return false;
-  if (B.isMemberDecorate()) {
-    auto &MDA = static_cast<SPIRVMemberDecorate const &>(A);
-    auto &MDB = static_cast<SPIRVMemberDecorate const &>(B);
-    if (MDA.getMemberNumber() != MDB.getMemberNumber())
-      return false;
-  }
-  if (A.getDecorateKind() != B.getDecorateKind())
-    return false;
-  if (A.getLiteralCount() != B.getLiteralCount())
-    return false;
-  for (size_t I = 0, E = A.getLiteralCount(); I != E; ++I) {
-    auto EA = A.getLiteral(I);
-    auto EB = B.getLiteral(I);
-    if (EA != EB)
-      return false;
-  }
-  return true;
-}
 } // namespace SPIRV

lib/SPIRV/libSPIRV/SPIRVDecorate.h

@@ -67,15 +67,6 @@ class SPIRVDecorateGeneric : public SPIRVAnnotationGeneric {
   std::vector<SPIRVWord> getVecLiteral() const;
   Decoration getDecorateKind() const;
   size_t getLiteralCount() const;
-  /// Compare for kind and literal only.
-  struct Comparator {
-    bool operator()(const SPIRVDecorateGeneric *A,
-                    const SPIRVDecorateGeneric *B) const;
-  };
-  /// Compare kind, literals and target.
-  friend bool operator==(const SPIRVDecorateGeneric &A,
-                         const SPIRVDecorateGeneric &B);
-
   SPIRVDecorationGroup *getOwner() const { return Owner; }
 
   void setOwner(SPIRVDecorationGroup *Owner) { this->Owner = Owner; }
@@ -117,26 +108,7 @@ class SPIRVDecorateGeneric : public SPIRVAnnotationGeneric {
   SPIRVDecorationGroup *Owner; // Owning decorate group
 };
 
-class SPIRVDecorateSet
-    : public std::multiset<SPIRVDecorateGeneric *,
-                           SPIRVDecorateGeneric::Comparator> {
-public:
-  typedef std::multiset<SPIRVDecorateGeneric *,
-                        SPIRVDecorateGeneric::Comparator>
-      BaseType;
-  iterator insert(value_type &Dec) {
-    auto ER = BaseType::equal_range(Dec);
-    for (auto I = ER.first, E = ER.second; I != E; ++I) {
-      SPIRVDBG(spvdbgs() << "[compare decorate] " << *Dec << " vs " << **I
-                         << " : ");
-      if (**I == *Dec)
-        return I;
-      SPIRVDBG(spvdbgs() << " diff\n");
-    }
-    SPIRVDBG(spvdbgs() << "[add decorate] " << *Dec << '\n');
-    return BaseType::insert(Dec);
-  }
-};
+typedef std::vector<SPIRVDecorateGeneric *> SPIRVDecorateVec;
 
 class SPIRVDecorate : public SPIRVDecorateGeneric {
 public:
@@ -378,17 +350,17 @@ class SPIRVDecorationGroup : public SPIRVEntry {
   void encodeAll(spv_ostream &O) const override;
   _SPIRV_DCL_ENCDEC
   // Move the given decorates to the decoration group
-  void takeDecorates(SPIRVDecorateSet &Decs) {
+  void takeDecorates(SPIRVDecorateVec &Decs) {
     Decorations = std::move(Decs);
     for (auto &I : Decorations)
       const_cast<SPIRVDecorateGeneric *>(I)->setOwner(this);
     Decs.clear();
   }
 
-  SPIRVDecorateSet &getDecorations() { return Decorations; }
+  SPIRVDecorateVec &getDecorations() { return Decorations; }
 
 protected:
-  SPIRVDecorateSet Decorations;
+  SPIRVDecorateVec Decorations;
   void validate() const override {
     assert(OpCode == OC);
     assert(WordCount == WC);

lib/SPIRV/libSPIRV/SPIRVModule.cpp

@@ -518,7 +518,7 @@ class SPIRVModuleImpl : public SPIRVModule {
   SPIRVStringVec StringVec;
   SPIRVMemberNameVec MemberNameVec;
   std::shared_ptr<const SPIRVLine> CurrentLine;
-  SPIRVDecorateSet DecorateSet;
+  SPIRVDecorateVec DecorateVec;
   SPIRVDecGroupVec DecGroupVec;
   SPIRVGroupDecVec GroupDecVec;
   SPIRVAsmTargetVector AsmTargetVec;
@@ -993,7 +993,7 @@ SPIRVModuleImpl::addDecorate(SPIRVDecorateGeneric *Dec) {
   (void)Found;
   assert(Found && "Decorate target does not exist");
   if (!Dec->getOwner())
-    DecorateSet.insert(Dec);
+    DecorateVec.push_back(Dec);
   addCapabilities(Dec->getRequiredCapability());
   return Dec;
 }
@@ -1678,8 +1678,8 @@ spv_ostream &operator<<(spv_ostream &O, const std::vector<T *> &V) {
   return O;
 }
 
-template <class T, class B>
-spv_ostream &operator<<(spv_ostream &O, const std::multiset<T *, B> &V) {
+template <class T, class B = std::less<T>>
+spv_ostream &operator<<(spv_ostream &O, const std::unordered_set<T *, B> &V) {
   for (auto &I : V)
     O << *I;
   return O;
@@ -1870,7 +1870,7 @@ spv_ostream &operator<<(spv_ostream &O, SPIRVModule &M) {
                      MI.ForwardPointerVec);
 
   O << MI.MemberNameVec << MI.ModuleProcessedVec << MI.DecGroupVec
-    << MI.DecorateSet << MI.GroupDecVec << MI.ForwardPointerVec << TS;
+    << MI.DecorateVec << MI.GroupDecVec << MI.ForwardPointerVec << TS;
 
   if (M.isAllowedToUseExtension(ExtensionID::SPV_INTEL_inline_assembly)) {
     O << SPIRVNL() << MI.AsmTargetVec << MI.AsmVec;
@@ -1895,11 +1895,11 @@ SPIRVDecorationGroup *SPIRVModuleImpl::addDecorationGroup() {
 SPIRVDecorationGroup *
 SPIRVModuleImpl::addDecorationGroup(SPIRVDecorationGroup *Group) {
   add(Group);
-  Group->takeDecorates(DecorateSet);
+  Group->takeDecorates(DecorateVec);
   DecGroupVec.push_back(Group);
   SPIRVDBG(spvdbgs() << "[addDecorationGroup] {" << *Group << "}\n";
-           spvdbgs() << "  Remaining DecorateSet: {" << DecorateSet << "}\n");
-  assert(DecorateSet.empty());
+           spvdbgs() << "  Remaining DecorateVec: {" << DecorateVec << "}\n");
+  assert(DecorateVec.empty());
   return Group;
 }
 

test/LinkOnceODR.ll

@@ -10,8 +10,8 @@
 
 ; CHECK-SPIRV: Capability Linkage
 ; CHECK-SPIRV: Extension "SPV_KHR_linkonce_odr"
-; CHECK-SPIRV: Decorate {{[0-9]+}} LinkageAttributes "GV" LinkOnceODR 
-; CHECK-SPIRV: Decorate {{[0-9]+}} LinkageAttributes "square" LinkOnceODR 
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} LinkageAttributes "GV" LinkOnceODR 
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} LinkageAttributes "square" LinkOnceODR 
 
 ; CHECK-SPIRV-NOEXT-NOT: Extension "SPV_KHR_linkonce_odr"
 ; CHECK-SPIRV-NOEXT-NOT: Decorate {{[0-9]+}} LinkageAttributes "GV" LinkOnceODR 

test/SpecConstants/long-spec-const-composite.ll

@@ -13,8 +13,8 @@ target triple = "spir64-unknown-unknown"
 
 ; CHECK-SPIRV: Capability LongConstantCompositeINTEL 
 ; CHECK-SPIRV: Extension "SPV_INTEL_long_constant_composite"
-; CHECK-SPIRV: Decorate [[First:[0-9]+]] SpecId  0
-; CHECK-SPIRV: Decorate [[Last:[0-9]+]] SpecId 65548
+; CHECK-SPIRV-DAG: Decorate [[First:[0-9]+]] SpecId  0
+; CHECK-SPIRV-DAG: Decorate [[Last:[0-9]+]] SpecId 65548
 ; CHECK-SPIRV: TypeInt [[TInt:[0-9]+]] 8
 ; CHECK-SPIRV: SpecConstant [[TInt]] [[First]]
 ; CHECK-SPIRV: SpecConstant [[TInt]] [[Last]]

test/builtin_vars-decorate.ll

@@ -26,23 +26,23 @@ target triple = "spir-unknown-unknown"
 ; CHECK: {{[0-9]+}} Name [[SI:[0-9]+]] "__spirv_BuiltInSubgroupId"
 ; CHECK: {{[0-9]+}} Name [[SLII:[0-9]+]] "__spirv_BuiltInSubgroupLocalInvocationId"
 
-; CHECK: 4 Decorate [[NW]] BuiltIn 24
-; CHECK: 4 Decorate [[WS]] BuiltIn 25
-; CHECK: 4 Decorate [[WI]] BuiltIn 26
-; CHECK: 4 Decorate [[LLI]] BuiltIn 27
-; CHECK: 4 Decorate [[GII]] BuiltIn 28
-; CHECK: 4 Decorate [[LLII]] BuiltIn 29
-; CHECK: 4 Decorate [[WD]] BuiltIn 30
-; CHECK: 4 Decorate [[GS]] BuiltIn 31
-; CHECK: 4 Decorate [[EWS]] BuiltIn 32
-; CHECK: 4 Decorate [[GO]] BuiltIn 33
-; CHECK: 4 Decorate [[GLI]] BuiltIn 34
-; CHECK: 4 Decorate [[SS]] BuiltIn 36
-; CHECK: 4 Decorate [[SMS]] BuiltIn 37
-; CHECK: 4 Decorate [[NS]] BuiltIn 38
-; CHECK: 4 Decorate [[NES]] BuiltIn 39
-; CHECK: 4 Decorate [[SI]] BuiltIn 40
-; CHECK: 4 Decorate [[SLII]] BuiltIn 41
+; CHECK-DAG: 4 Decorate [[NW]] BuiltIn 24
+; CHECK-DAG: 4 Decorate [[WS]] BuiltIn 25
+; CHECK-DAG: 4 Decorate [[WI]] BuiltIn 26
+; CHECK-DAG: 4 Decorate [[LLI]] BuiltIn 27
+; CHECK-DAG: 4 Decorate [[GII]] BuiltIn 28
+; CHECK-DAG: 4 Decorate [[LLII]] BuiltIn 29
+; CHECK-DAG: 4 Decorate [[WD]] BuiltIn 30
+; CHECK-DAG: 4 Decorate [[GS]] BuiltIn 31
+; CHECK-DAG: 4 Decorate [[EWS]] BuiltIn 32
+; CHECK-DAG: 4 Decorate [[GO]] BuiltIn 33
+; CHECK-DAG: 4 Decorate [[GLI]] BuiltIn 34
+; CHECK-DAG: 4 Decorate [[SS]] BuiltIn 36
+; CHECK-DAG: 4 Decorate [[SMS]] BuiltIn 37
+; CHECK-DAG: 4 Decorate [[NS]] BuiltIn 38
+; CHECK-DAG: 4 Decorate [[NES]] BuiltIn 39
+; CHECK-DAG: 4 Decorate [[SI]] BuiltIn 40
+; CHECK-DAG: 4 Decorate [[SLII]] BuiltIn 41
 @__spirv_BuiltInWorkDim = external addrspace(1) global i32
 @__spirv_BuiltInGlobalSize = external addrspace(1) global <3 x i32>
 @__spirv_BuiltInGlobalInvocationId = external addrspace(1) global <3 x i32>

test/float-controls-decorations.ll

@@ -23,24 +23,24 @@ entry:
 ; CHECK-LLVM: "VCFloatControl"="48"
 ; CHECK-SPIRV: 3 Name [[#FOO_ID:]] "foo"
 ; CHECK-SPIRV: 3 Name [[#BAR_ID:]] "bar"
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 16 0
-; CHECK-SPIRV-NEXT: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 16 1
-; CHECK-SPIRV-NEXT: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 32 0
-; CHECK-SPIRV-NEXT: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 32 1
-; CHECK-SPIRV-NEXT: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 64 0
-; CHECK-SPIRV-NEXT: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 64 1
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 16 1
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 16 1
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 32 1
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 32 1
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 64 1
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 64 1
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 16 0
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 16 0
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 32 0
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 32 0
-; CHECK-SPIRV: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 64 0
-; CHECK-SPIRV: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 64 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 16 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 16 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 32 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 32 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionRoundingModeINTEL 64 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionRoundingModeINTEL 64 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 16 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 16 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 32 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 32 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionDenormModeINTEL 64 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionDenormModeINTEL 64 1
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 16 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 16 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 32 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 32 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#FOO_ID]] FunctionFloatingPointModeINTEL 64 0
+; CHECK-SPIRV-DAG: 5 Decorate [[#BAR_ID]] FunctionFloatingPointModeINTEL 64 0
 
 attributes #0 = { "VCFloatControl"="0" "VCFunction"  }
 attributes #1 = { "VCFloatControl"="48" "VCFunction" }

test/llvm-intrinsics/constrained-arithmetic.ll

@@ -11,10 +11,10 @@
 
 ; CHECK-NOT: Decorate {{[0-9]+}} FPRoundingMode
 
-; CHECK: Decorate [[ad]] FPRoundingMode 0
-; CHECK: Decorate [[di]] FPRoundingMode 1
-; CHECK: Decorate [[su]] FPRoundingMode 2
-; CHECK: Decorate [[mu]] FPRoundingMode 3
+; CHECK-DAG: Decorate [[ad]] FPRoundingMode 0
+; CHECK-DAG: Decorate [[di]] FPRoundingMode 1
+; CHECK-DAG: Decorate [[su]] FPRoundingMode 2
+; CHECK-DAG: Decorate [[mu]] FPRoundingMode 3
 
 ; CHECK-NOT: Decorate {{[0-9]+}} FPRoundingMode
 

test/llvm-intrinsics/constrained-convert.ll

@@ -10,14 +10,14 @@
 ; CHECK: Name [[fe:[0-9]+]] "conv4"
 ; CHECK: Name [[ft:[0-9]+]] "conv5"
 
-; CHECK: Decorate [[sf]] FPRoundingMode 0
-; CHECK: Decorate [[uf]] FPRoundingMode 1
+; CHECK-DAG: Decorate [[sf]] FPRoundingMode 0
+; CHECK-DAG: Decorate [[uf]] FPRoundingMode 1
+; CHECK-DAG: Decorate [[ft]] FPRoundingMode 2
 
 ; CHECK-NOT: Decorate [[fs]] FPRoundingMode
 ; CHECK-NOT: Decorate [[fu]] FPRoundingMode
 ; CHECK-NOT: Decorate [[fe]] FPRoundingMode
 
-; CHECK: Decorate [[ft]] FPRoundingMode 2
 
 ;CHECK: ConvertSToF {{[0-9]+}} [[sf]]
 ;CHECK: ConvertUToF {{[0-9]+}} [[uf]]

test/transcoding/DecorationMaxByteOffset.ll

@@ -11,8 +11,8 @@
 
 ; CHECK-SPIRV: 3 Name [[PTR_ID:[0-9]+]] "ptr"
 ; CHECK-SPIRV: 4 Name [[PTR2_ID:[0-9]+]] "ptr2"
-; CHECK-SPIRV: 4 Decorate [[PTR_ID]] MaxByteOffset 12
-; CHECK-SPIRV: 4 Decorate [[PTR2_ID]] MaxByteOffset 123
+; CHECK-SPIRV-DAG: 4 Decorate [[PTR_ID]] MaxByteOffset 12
+; CHECK-SPIRV-DAG: 4 Decorate [[PTR2_ID]] MaxByteOffset 123
 ; CHECK-SPIRV: 4 TypeInt [[CHAR_T:[0-9]+]] 8 0
 ; CHECK-SPIRV: 4 TypePointer [[CHAR_PTR_T:[0-9]+]] 4 [[CHAR_T]]
 ; CHECK-SPIRV: 3 FunctionParameter [[CHAR_PTR_T]] [[PTR_ID]]

test/transcoding/IntelFPGAMemoryAccesses.ll

@@ -52,12 +52,12 @@
 ; CHECK-SPIRV: Extension "SPV_INTEL_fpga_memory_accesses"
 ; Check that the semantically meaningless decoration was
 ; translated as a mere annotation
-; CHECK-SPIRV: Decorate {{[0-9]+}} UserSemantic "{params:0}{cache-size:127}"
-; CHECK-SPIRV: Decorate {{[0-9]+}} BurstCoalesceINTEL
-; CHECK-SPIRV: Decorate {{[0-9]+}} CacheSizeINTEL 0
-; CHECK-SPIRV: Decorate {{[0-9]+}} CacheSizeINTEL 127
-; CHECK-SPIRV: Decorate {{[0-9]+}} DontStaticallyCoalesceINTEL
-; CHECK-SPIRV: Decorate {{[0-9]+}} PrefetchINTEL 0
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} UserSemantic "{params:0}{cache-size:127}"
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} BurstCoalesceINTEL
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} CacheSizeINTEL 0
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} CacheSizeINTEL 127
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} DontStaticallyCoalesceINTEL
+; CHECK-SPIRV-DAG: Decorate {{[0-9]+}} PrefetchINTEL 0
 
 target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
 target triple = "spir64-unknown-unknown"