[Mlir-commits] [mlir] 74d44c4 - [mlir] Refactor and cleanup the translation facilities.

River Riddle llvmlistbot at llvm.org
Sun Apr 5 16:25:57 PDT 2020


Author: River Riddle
Date: 2020-04-05T16:21:21-07:00
New Revision: 74d44c43e8caa5d325106b0587ea030360ed01c9

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

LOG: [mlir] Refactor and cleanup the translation facilities.

Summary:
This revision performs several cleanups on the translation infra:
* Removes the TranslateCLParser library and consolidates into Translation
  - This was a weird library that existed in Support, and didn't really justify being a standalone library.
* Cleans up the internal registration and consolidates all of the translation functions within one registry.

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

Added: 
    

Modified: 
    mlir/include/mlir/Translation.h
    mlir/lib/Support/CMakeLists.txt
    mlir/lib/Translation/CMakeLists.txt
    mlir/lib/Translation/Translation.cpp
    mlir/tools/mlir-translate/CMakeLists.txt
    mlir/tools/mlir-translate/mlir-translate.cpp

Removed: 
    mlir/include/mlir/Support/TranslateClParser.h
    mlir/lib/Support/TranslateClParser.cpp


################################################################################
diff  --git a/mlir/include/mlir/Support/TranslateClParser.h b/mlir/include/mlir/Support/TranslateClParser.h
deleted file mode 100644
index 1e15079aca21..000000000000
--- a/mlir/include/mlir/Support/TranslateClParser.h
+++ /dev/null
@@ -1,38 +0,0 @@
-//===- TranslateClParser.h - Translations command line parser ---*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains custom command line parser for translations.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef MLIR_SUPPORT_TRANSLATE_CL_PARSER_H_
-#define MLIR_SUPPORT_TRANSLATE_CL_PARSER_H_
-
-#include "mlir/Support/LLVM.h"
-#include "mlir/Translation.h"
-#include "llvm/Support/CommandLine.h"
-#include <functional>
-
-namespace mlir {
-
-struct LogicalResult;
-class MLIRContext;
-
-/// Custom parser for TranslateFunction.
-/// Wraps TranslateToMLIRFunctions and TranslateFromMLIRFunctions into
-/// TranslateFunctions before registering them as options.
-struct TranslationParser : public llvm::cl::parser<const TranslateFunction *> {
-  TranslationParser(llvm::cl::Option &opt);
-
-  void printOptionInfo(const llvm::cl::Option &O,
-                       size_t GlobalWidth) const override;
-};
-
-} // namespace mlir
-
-#endif // MLIR_SUPPORT_TRANSLATE_CL_PARSER_H_

diff  --git a/mlir/include/mlir/Translation.h b/mlir/include/mlir/Translation.h
index 10cfb95c9a4f..72cbee8baf58 100644
--- a/mlir/include/mlir/Translation.h
+++ b/mlir/include/mlir/Translation.h
@@ -12,9 +12,7 @@
 #ifndef MLIR_TRANSLATION_H
 #define MLIR_TRANSLATION_H
 
-#include "llvm/ADT/StringMap.h"
-
-#include <memory>
+#include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 class MemoryBuffer;
@@ -82,13 +80,13 @@ struct TranslateRegistration {
 };
 /// \}
 
-/// Get a read-only reference to the translator registry.
-const llvm::StringMap<TranslateSourceMgrToMLIRFunction> &
-getTranslationToMLIRRegistry();
-const llvm::StringMap<TranslateFromMLIRFunction> &
-getTranslationFromMLIRRegistry();
-const llvm::StringMap<TranslateFunction> &getTranslationRegistry();
+/// A command line parser for translation functions.
+struct TranslationParser : public llvm::cl::parser<const TranslateFunction *> {
+  TranslationParser(llvm::cl::Option &opt);
 
+  void printOptionInfo(const llvm::cl::Option &o,
+                       size_t globalWidth) const override;
+};
 } // namespace mlir
 
 #endif // MLIR_TRANSLATION_H

diff  --git a/mlir/lib/Support/CMakeLists.txt b/mlir/lib/Support/CMakeLists.txt
index 6f34e7a7ddf3..a21a8cc29e0f 100644
--- a/mlir/lib/Support/CMakeLists.txt
+++ b/mlir/lib/Support/CMakeLists.txt
@@ -4,7 +4,6 @@ set(LLVM_OPTIONAL_SOURCES
   MlirOptMain.cpp
   StorageUniquer.cpp
   ToolUtilities.cpp
-  TranslateClParser.cpp
 )
 
 add_mlir_library(MLIRSupport
@@ -34,19 +33,6 @@ target_link_libraries(MLIROptLib
   MLIRSupport
   )
 
-add_mlir_library(MLIRTranslateClParser
-  TranslateClParser.cpp
-
-  ADDITIONAL_HEADER_DIRS
-  ${MLIR_MAIN_INCLUDE_DIR}/mlir/Support
-  )
-target_link_libraries(MLIRTranslateClParser
-  PUBLIC
-  LLVMSupport
-  MLIRIR
-  MLIRTranslation
-  MLIRParser)
-
 add_llvm_library(MLIRJitRunner
   JitRunner.cpp
 )

diff  --git a/mlir/lib/Support/TranslateClParser.cpp b/mlir/lib/Support/TranslateClParser.cpp
deleted file mode 100644
index ea2fd6a52283..000000000000
--- a/mlir/lib/Support/TranslateClParser.cpp
+++ /dev/null
@@ -1,93 +0,0 @@
-//===- TranslateClParser.h - Translations command line parser -------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file contains custom command line parser for translations.
-//
-//===----------------------------------------------------------------------===//
-
-#include "mlir/Support/TranslateClParser.h"
-
-#include "mlir/Analysis/Verifier.h"
-#include "mlir/IR/MLIRContext.h"
-#include "mlir/IR/Module.h"
-#include "mlir/Parser.h"
-#include "mlir/Support/LogicalResult.h"
-#include "mlir/Translation.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileUtilities.h"
-#include "llvm/Support/SourceMgr.h"
-#include "llvm/Support/ToolOutputFile.h"
-
-using namespace mlir;
-
-// Storage for the translation function wrappers that survive the parser.
-static SmallVector<TranslateFunction, 16> wrapperStorage;
-
-static LogicalResult printMLIROutput(ModuleOp module, raw_ostream &os) {
-  if (failed(verify(module)))
-    return failure();
-  module.print(os);
-  return success();
-}
-
-TranslationParser::TranslationParser(llvm::cl::Option &opt)
-    : llvm::cl::parser<const TranslateFunction *>(opt) {
-  const auto &toMLIRRegistry = getTranslationToMLIRRegistry();
-  const auto &fromMLIRRegistry = getTranslationFromMLIRRegistry();
-  const auto &fileToFileRegistry = getTranslationRegistry();
-
-  // Reserve the required capacity upfront so that pointers are not
-  // invalidated on reallocation.
-  wrapperStorage.reserve(toMLIRRegistry.size() + fromMLIRRegistry.size() +
-                         fileToFileRegistry.size());
-  for (const auto &kv : toMLIRRegistry) {
-    TranslateSourceMgrToMLIRFunction function = kv.second;
-    TranslateFunction wrapper = [function](llvm::SourceMgr &sourceMgr,
-                                           raw_ostream &output,
-                                           MLIRContext *context) {
-      OwningModuleRef module = function(sourceMgr, context);
-      if (!module)
-        return failure();
-      return printMLIROutput(*module, output);
-    };
-    wrapperStorage.emplace_back(std::move(wrapper));
-
-    addLiteralOption(kv.first(), &wrapperStorage.back(), kv.first());
-  }
-
-  for (const auto &kv : fromMLIRRegistry) {
-    TranslateFromMLIRFunction function = kv.second;
-    TranslateFunction wrapper = [function](llvm::SourceMgr &sourceMgr,
-                                           raw_ostream &output,
-                                           MLIRContext *context) {
-      auto module = OwningModuleRef(parseSourceFile(sourceMgr, context));
-      if (!module)
-        return failure();
-      return function(module.get(), output);
-    };
-    wrapperStorage.emplace_back(std::move(wrapper));
-
-    addLiteralOption(kv.first(), &wrapperStorage.back(), kv.first());
-  }
-  for (const auto &kv : fileToFileRegistry) {
-    wrapperStorage.emplace_back(kv.second);
-    addLiteralOption(kv.first(), &wrapperStorage.back(), kv.first());
-  }
-}
-
-void TranslationParser::printOptionInfo(const llvm::cl::Option &O,
-                                        size_t GlobalWidth) const {
-  TranslationParser *TP = const_cast<TranslationParser *>(this);
-  llvm::array_pod_sort(TP->Values.begin(), TP->Values.end(),
-                       [](const TranslationParser::OptionInfo *VT1,
-                          const TranslationParser::OptionInfo *VT2) {
-                         return VT1->Name.compare(VT2->Name);
-                       });
-  using llvm::cl::parser;
-  parser<const TranslateFunction *>::printOptionInfo(O, GlobalWidth);
-}

diff  --git a/mlir/lib/Translation/CMakeLists.txt b/mlir/lib/Translation/CMakeLists.txt
index 8b999d26987d..2cd1a7c9ee3e 100644
--- a/mlir/lib/Translation/CMakeLists.txt
+++ b/mlir/lib/Translation/CMakeLists.txt
@@ -8,4 +8,5 @@ target_link_libraries(MLIRTranslation
   PUBLIC
   LLVMSupport
   MLIRIR
+  MLIRParser
   )

diff  --git a/mlir/lib/Translation/Translation.cpp b/mlir/lib/Translation/Translation.cpp
index 01ba689a932b..487e39ac939c 100644
--- a/mlir/lib/Translation/Translation.cpp
+++ b/mlir/lib/Translation/Translation.cpp
@@ -11,46 +11,59 @@
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Translation.h"
+#include "mlir/Analysis/Verifier.h"
 #include "mlir/IR/Module.h"
+#include "mlir/Parser.h"
 #include "mlir/Support/LLVM.h"
-#include "mlir/Support/LogicalResult.h"
 #include "llvm/Support/SourceMgr.h"
 
 using namespace mlir;
 
-// Get the mutable static map between registered "to MLIR" translations and the
-// TranslateToMLIRFunctions that perform those translations.
-static llvm::StringMap<TranslateSourceMgrToMLIRFunction> &
-getMutableTranslationToMLIRRegistry() {
-  static llvm::StringMap<TranslateSourceMgrToMLIRFunction>
-      translationToMLIRRegistry;
-  return translationToMLIRRegistry;
-}
-// Get the mutable static map between registered "from MLIR" translations and
-// the TranslateFromMLIRFunctions that perform those translations.
-static llvm::StringMap<TranslateFromMLIRFunction> &
-getMutableTranslationFromMLIRRegistry() {
-  static llvm::StringMap<TranslateFromMLIRFunction> translationFromMLIRRegistry;
-  return translationFromMLIRRegistry;
-}
+//===----------------------------------------------------------------------===//
+// Translation Registry
+//===----------------------------------------------------------------------===//
 
-// Get the mutable static map between registered file-to-file MLIR translations
-// and the TranslateFunctions that perform those translations.
-static llvm::StringMap<TranslateFunction> &getMutableTranslationRegistry() {
+/// Get the mutable static map between registered file-to-file MLIR translations
+/// and the TranslateFunctions that perform those translations.
+static llvm::StringMap<TranslateFunction> &getTranslationRegistry() {
   static llvm::StringMap<TranslateFunction> translationRegistry;
   return translationRegistry;
 }
 
+/// Register the given translation.
+static void registerTranslation(StringRef name,
+                                const TranslateFunction &function) {
+  auto &translationRegistry = getTranslationRegistry();
+  if (translationRegistry.find(name) != translationRegistry.end())
+    llvm::report_fatal_error(
+        "Attempting to overwrite an existing <file-to-file> function");
+  assert(function &&
+         "Attempting to register an empty translate <file-to-file> function");
+  translationRegistry[name] = function;
+}
+
+TranslateRegistration::TranslateRegistration(
+    StringRef name, const TranslateFunction &function) {
+  registerTranslation(name, function);
+}
+
+//===----------------------------------------------------------------------===//
+// Translation to MLIR
+//===----------------------------------------------------------------------===//
+
 // Puts `function` into the to-MLIR translation registry unless there is already
 // a function registered for the same name.
 static void registerTranslateToMLIRFunction(
     StringRef name, const TranslateSourceMgrToMLIRFunction &function) {
-  auto &translationToMLIRRegistry = getMutableTranslationToMLIRRegistry();
-  if (translationToMLIRRegistry.find(name) != translationToMLIRRegistry.end())
-    llvm::report_fatal_error(
-        "Attempting to overwrite an existing <to> function");
-  assert(function && "Attempting to register an empty translate <to> function");
-  translationToMLIRRegistry[name] = function;
+  auto wrappedFn = [function](llvm::SourceMgr &sourceMgr, raw_ostream &output,
+                              MLIRContext *context) {
+    OwningModuleRef module = function(sourceMgr, context);
+    if (!module || failed(verify(*module)))
+      return failure();
+    module->print(output);
+    return success();
+  };
+  registerTranslation(name, wrappedFn);
 }
 
 TranslateToMLIRRegistration::TranslateToMLIRRegistration(
@@ -58,54 +71,51 @@ TranslateToMLIRRegistration::TranslateToMLIRRegistration(
   registerTranslateToMLIRFunction(name, function);
 }
 
-// Wraps `function` with a lambda that extracts a StringRef from a source
-// manager and registers the wrapper lambda as a to-MLIR conversion.
+/// Wraps `function` with a lambda that extracts a StringRef from a source
+/// manager and registers the wrapper lambda as a to-MLIR conversion.
 TranslateToMLIRRegistration::TranslateToMLIRRegistration(
     StringRef name, const TranslateStringRefToMLIRFunction &function) {
-  auto translationFunction = [function](llvm::SourceMgr &sourceMgr,
-                                        MLIRContext *ctx) {
-    const llvm::MemoryBuffer *buffer =
-        sourceMgr.getMemoryBuffer(sourceMgr.getMainFileID());
-    return function(buffer->getBuffer(), ctx);
-  };
-  registerTranslateToMLIRFunction(name, translationFunction);
+  registerTranslateToMLIRFunction(
+      name, [function](llvm::SourceMgr &sourceMgr, MLIRContext *ctx) {
+        const llvm::MemoryBuffer *buffer =
+            sourceMgr.getMemoryBuffer(sourceMgr.getMainFileID());
+        return function(buffer->getBuffer(), ctx);
+      });
 }
 
+//===----------------------------------------------------------------------===//
+// Translation from MLIR
+//===----------------------------------------------------------------------===//
+
 TranslateFromMLIRRegistration::TranslateFromMLIRRegistration(
     StringRef name, const TranslateFromMLIRFunction &function) {
-  auto &translationFromMLIRRegistry = getMutableTranslationFromMLIRRegistry();
-  if (translationFromMLIRRegistry.find(name) !=
-      translationFromMLIRRegistry.end())
-    llvm::report_fatal_error(
-        "Attempting to overwrite an existing <from> function");
-  assert(function &&
-         "Attempting to register an empty translate <from> function");
-  translationFromMLIRRegistry[name] = function;
-}
-
-TranslateRegistration::TranslateRegistration(
-    StringRef name, const TranslateFunction &function) {
-  auto &translationRegistry = getMutableTranslationRegistry();
-  if (translationRegistry.find(name) != translationRegistry.end())
-    llvm::report_fatal_error(
-        "Attempting to overwrite an existing <file-to-file> function");
-  assert(function &&
-         "Attempting to register an empty translate <file-to-file> function");
-  translationRegistry[name] = function;
+  registerTranslation(name, [function](llvm::SourceMgr &sourceMgr,
+                                       raw_ostream &output,
+                                       MLIRContext *context) {
+    auto module = OwningModuleRef(parseSourceFile(sourceMgr, context));
+    if (!module)
+      return failure();
+    return function(module.get(), output);
+  });
 }
 
-// Merely add the const qualifier to the mutable registry so that external users
-// cannot modify it.
-const llvm::StringMap<TranslateSourceMgrToMLIRFunction> &
-mlir::getTranslationToMLIRRegistry() {
-  return getMutableTranslationToMLIRRegistry();
-}
+//===----------------------------------------------------------------------===//
+// Translation Parser
+//===----------------------------------------------------------------------===//
 
-const llvm::StringMap<TranslateFromMLIRFunction> &
-mlir::getTranslationFromMLIRRegistry() {
-  return getMutableTranslationFromMLIRRegistry();
+TranslationParser::TranslationParser(llvm::cl::Option &opt)
+    : llvm::cl::parser<const TranslateFunction *>(opt) {
+  for (const auto &kv : getTranslationRegistry())
+    addLiteralOption(kv.first(), &kv.second, kv.first());
 }
 
-const llvm::StringMap<TranslateFunction> &mlir::getTranslationRegistry() {
-  return getMutableTranslationRegistry();
+void TranslationParser::printOptionInfo(const llvm::cl::Option &o,
+                                        size_t globalWidth) const {
+  TranslationParser *tp = const_cast<TranslationParser *>(this);
+  llvm::array_pod_sort(tp->Values.begin(), tp->Values.end(),
+                       [](const TranslationParser::OptionInfo *lhs,
+                          const TranslationParser::OptionInfo *rhs) {
+                         return lhs->Name.compare(rhs->Name);
+                       });
+  llvm::cl::parser<const TranslateFunction *>::printOptionInfo(o, globalWidth);
 }

diff  --git a/mlir/tools/mlir-translate/CMakeLists.txt b/mlir/tools/mlir-translate/CMakeLists.txt
index bf7a92509912..ff813245f7c8 100644
--- a/mlir/tools/mlir-translate/CMakeLists.txt
+++ b/mlir/tools/mlir-translate/CMakeLists.txt
@@ -24,4 +24,4 @@ add_llvm_tool(mlir-translate
 )
 llvm_update_compile_flags(mlir-translate)
 whole_archive_link(mlir-translate ${FULL_LIBS})
-target_link_libraries(mlir-translate PRIVATE MLIRIR MLIRTranslateClParser ${LIBS} LLVMSupport)
+target_link_libraries(mlir-translate PRIVATE MLIRIR MLIRTranslation ${LIBS} LLVMSupport)

diff  --git a/mlir/tools/mlir-translate/mlir-translate.cpp b/mlir/tools/mlir-translate/mlir-translate.cpp
index d7c1eb626952..c9240bb6b20c 100644
--- a/mlir/tools/mlir-translate/mlir-translate.cpp
+++ b/mlir/tools/mlir-translate/mlir-translate.cpp
@@ -17,7 +17,7 @@
 #include "mlir/Support/FileUtilities.h"
 #include "mlir/Support/LogicalResult.h"
 #include "mlir/Support/ToolUtilities.h"
-#include "mlir/Support/TranslateClParser.h"
+#include "mlir/Translation.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"


        


More information about the Mlir-commits mailing list