[Mlir-commits] [llvm] [mlir] [MLIR][TableGen] Minor code cleanup in DirectiveCommonGen (PR #110290)

[Rahul Joshi]

https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/110290

>From ae7490cb46bf71684eb921cbe53ad7e2aed3dfbe Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Fri, 27 Sep 2024 09:06:22 -0700
Subject: [PATCH] [MLIR][TableGen] Minor code cleanup in DirectiveCommonGen

Directly Use Clause/ClauseVal as loop iterator.
Use llvm::transform instead of std::transform.
Use ListSeparator to generate , separated list.
---
 llvm/include/llvm/TableGen/DirectiveEmitter.h |  2 +-
 llvm/utils/TableGen/DirectiveEmitter.cpp      |  6 ++----
 mlir/test/mlir-tblgen/directive-common.td     |  2 +-
 mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp | 18 ++++++++----------
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/llvm/include/llvm/TableGen/DirectiveEmitter.h b/llvm/include/llvm/TableGen/DirectiveEmitter.h
index 014a2bc4b88313..70d7d8fbe14f83 100644
--- a/llvm/include/llvm/TableGen/DirectiveEmitter.h
+++ b/llvm/include/llvm/TableGen/DirectiveEmitter.h
@@ -247,7 +247,7 @@ class VersionedClause {
 
 class ClauseVal : public BaseRecord {
 public:
-  explicit ClauseVal(const Record *Def) : BaseRecord(Def) {}
+  ClauseVal(const Record *Def) : BaseRecord(Def) {}
 
   int getValue() const { return Def->getValueAsInt("value"); }
 
diff --git a/llvm/utils/TableGen/DirectiveEmitter.cpp b/llvm/utils/TableGen/DirectiveEmitter.cpp
index dc9ed2b08972e2..9dc29d8262fa2c 100644
--- a/llvm/utils/TableGen/DirectiveEmitter.cpp
+++ b/llvm/utils/TableGen/DirectiveEmitter.cpp
@@ -98,10 +98,8 @@ static void GenerateEnumClauseVal(ArrayRef<const Record *> Records,
 
     OS << "\n";
     OS << "enum class " << EnumName << " {\n";
-    for (const auto &CV : ClauseVals) {
-      ClauseVal CVal(CV);
-      OS << "  " << CV->getName() << "=" << CVal.getValue() << ",\n";
-    }
+    for (const ClauseVal CVal : ClauseVals)
+      OS << "  " << CVal.getRecordName() << "=" << CVal.getValue() << ",\n";
     OS << "};\n";
 
     if (DirLang.hasMakeEnumAvailableInNamespace()) {
diff --git a/mlir/test/mlir-tblgen/directive-common.td b/mlir/test/mlir-tblgen/directive-common.td
index dd86dea36417c0..9429238a03f075 100644
--- a/mlir/test/mlir-tblgen/directive-common.td
+++ b/mlir/test/mlir-tblgen/directive-common.td
@@ -26,7 +26,7 @@ def TDLC_ClauseA : Clause<"clausea"> {
 // CHECK: def AKind: I32EnumAttr<
 // CHECK:   "ClauseAKind",
 // CHECK:   "AKind Clause",
-// CHECK:   [AKindvala,AKindvalb]> {
+// CHECK:   [AKindvala, AKindvalb]> {
 // CHECK:     let cppNamespace = "::mlir::tdl";
 // CHECK: }
 // CHECK: def AKindAttr : EnumAttr<TDL_Dialect, AKind, "akind">;
diff --git a/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp b/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
index de3e6d8ee8cbc8..aaed481b3efb0c 100644
--- a/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
+++ b/mlir/tools/mlir-tblgen/DirectiveCommonGen.cpp
@@ -13,6 +13,7 @@
 
 #include "mlir/TableGen/GenInfo.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -25,6 +26,7 @@ using llvm::ClauseVal;
 using llvm::raw_ostream;
 using llvm::Record;
 using llvm::RecordKeeper;
+using llvm::Twine;
 
 // LLVM has multiple places (Clang, Flang, MLIR) where information about
 // the directives (OpenMP/OpenACC), and clauses are needed. It is good software
@@ -54,8 +56,7 @@ static bool emitDecls(const RecordKeeper &recordKeeper, llvm::StringRef dialect,
       recordKeeper.getAllDerivedDefinitions("DirectiveLanguage");
   assert(!directiveLanguages.empty() && "DirectiveLanguage missing.");
 
-  for (const Record *r : recordKeeper.getAllDerivedDefinitions("Clause")) {
-    Clause c{r};
+  for (const Clause c : recordKeeper.getAllDerivedDefinitions("Clause")) {
     const auto &clauseVals = c.getClauseVals();
     if (clauseVals.empty())
       continue;
@@ -65,14 +66,13 @@ static bool emitDecls(const RecordKeeper &recordKeeper, llvm::StringRef dialect,
 
     std::vector<std::string> cvDefs;
     for (const auto &it : llvm::enumerate(clauseVals)) {
-      ClauseVal cval{it.value()};
+      const ClauseVal cval{it.value()};
       if (!cval.isUserVisible())
         continue;
 
       std::string name = cval.getFormattedName();
       std::string enumValName(name.length(), ' ');
-      std::transform(name.begin(), name.end(), enumValName.begin(),
-                     llvm::toLower);
+      llvm::transform(name, enumValName.begin(), llvm::toLower);
       enumValName[0] = llvm::toUpper(enumValName[0]);
       std::string cvDef{(enumName + llvm::Twine(name)).str()};
       os << "def " << cvDef << " : I32EnumAttrCase<\"" << enumValName << "\", "
@@ -84,11 +84,9 @@ static bool emitDecls(const RecordKeeper &recordKeeper, llvm::StringRef dialect,
     os << "  \"Clause" << enumName << "\",\n";
     os << "  \"" << enumName << " Clause\",\n";
     os << "  [";
-    for (unsigned int i = 0; i < cvDefs.size(); i++) {
-      os << cvDefs[i];
-      if (i != cvDefs.size() - 1)
-        os << ",";
-    }
+    llvm::ListSeparator LS;
+    for (const auto &cvDef : cvDefs)
+      os << LS << cvDef;
     os << "]> {\n";
     os << "    let cppNamespace = \"::mlir::"
        << directiveLanguages[0]->getValueAsString("cppNamespace") << "\";\n";