[PATCH] D62119: [RISCV] prefer std::set to std::map

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: sabuasal, asb.
Herald added subscribers: llvm-commits, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, apazos, simoncook, johnrusso, rbar.
Herald added a project: LLVM.

This was flagged in https://www.viva64.com/en/b/0629/ under "Snippet No.
21".

It seems that a std::map is created, but only the keys are ever used.

 ______________________________________

< It looks like you meant to use a set >

 --------------------------------------

\

  \
     __
    /  \
    |  |
    @  @
    |  |
    || |/
    || ||
    |\_/|
    \___/

Yes we did! Thanks Clippy! (yay!)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62119

Files:
  llvm/utils/TableGen/RISCVCompressInstEmitter.cpp


Index: llvm/utils/TableGen/RISCVCompressInstEmitter.cpp
===================================================================
--- llvm/utils/TableGen/RISCVCompressInstEmitter.cpp
+++ llvm/utils/TableGen/RISCVCompressInstEmitter.cpp
@@ -64,6 +64,7 @@
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
+#include <set>
 #include <vector>
 using namespace llvm;
 
@@ -474,7 +475,7 @@
                                          SourceOperandMap, DestOperandMap));
 }
 
-static void getReqFeatures(std::map<StringRef, int> &FeaturesMap,
+static void getReqFeatures(std::set<StringRef> &Features,
                            const std::vector<Record *> &ReqFeatures) {
   for (auto &R : ReqFeatures) {
     StringRef AsmCondString = R->getValueAsString("AssemblerCondString");
@@ -486,8 +487,7 @@
 
     for (auto &Op : Ops) {
       assert(!Op.empty() && "Empty operator");
-      if (FeaturesMap.find(Op) == FeaturesMap.end())
-        FeaturesMap[Op] = FeaturesMap.size();
+      Features.emplace(Op);
     }
   }
 }
@@ -620,30 +620,32 @@
       CaseStream.indent(4) << "case " + Namespace + "::" + CurOp + ": {\n";
     }
 
-    std::map<StringRef, int> FeaturesMap;
-    // Add CompressPat required features.
-    getReqFeatures(FeaturesMap, CompressPat.PatReqFeatures);
-
-    // Add Dest instruction required features.
-    std::vector<Record *> ReqFeatures;
-    std::vector<Record *> RF = Dest.TheDef->getValueAsListOfDefs("Predicates");
-    copy_if(RF, std::back_inserter(ReqFeatures), [](Record *R) {
-      return R->getValueAsBit("AssemblerMatcherPredicate");
-    });
-    getReqFeatures(FeaturesMap, ReqFeatures);
-
-    // Emit checks for all required features.
-    for (auto &F : FeaturesMap) {
-      StringRef Op = F.first;
-      if (Op[0] == '!')
-        CondStream.indent(6) << ("!STI.getFeatureBits()[" + Namespace +
-                                 "::" + Op.substr(1) + "]")
-                                        .str() +
-                                    " &&\n";
-      else
-        CondStream.indent(6)
-            << ("STI.getFeatureBits()[" + Namespace + "::" + Op + "]").str() +
-                   " &&\n";
+    {
+      std::set<StringRef> Features;
+      // Add CompressPat required features.
+      getReqFeatures(Features, CompressPat.PatReqFeatures);
+
+      // Add Dest instruction required features.
+      std::vector<Record *> ReqFeatures;
+      std::vector<Record *> RF =
+          Dest.TheDef->getValueAsListOfDefs("Predicates");
+      copy_if(RF, std::back_inserter(ReqFeatures), [](Record *R) {
+        return R->getValueAsBit("AssemblerMatcherPredicate");
+      });
+      getReqFeatures(Features, ReqFeatures);
+
+      // Emit checks for all required features.
+      for (const StringRef &Op : Features) {
+        if (Op[0] == '!')
+          CondStream.indent(6) << ("!STI.getFeatureBits()[" + Namespace +
+                                   "::" + Op.substr(1) + "]")
+                                          .str() +
+                                      " &&\n";
+        else
+          CondStream.indent(6)
+              << ("STI.getFeatureBits()[" + Namespace + "::" + Op + "]").str() +
+                     " &&\n";
+      }
     }
 
     // Start Source Inst operands validation.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62119.200200.patch
Type: text/x-patch
Size: 3316 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190520/92729e56/attachment.bin>