[mlir] Revise naming of MLIROptMain and MLIRMlirOptLib

[marbre]

Summary

• Rename CMake target MLIROptMain to MLIROptLib: The target provides the main library
• Rename CMake target MLIRMlirOptLib to MLIRMlirOptMain: The target provides the main() entry function

At the moment, the Bazel configuration of TenorFlow maps the target MlirOptLib to "lib/Support/MlirOptMain.cpp" and MlirOptMain to "tools/mlir-opt/mlir-opt.cpp". This is the other way around in the CMake configuration. As discussed in the context of the pull request https://github.com/tensorflow/tensorflow/pull/36301, it seems useful to revise the naming in the MLIR repo.

[merge_guards_bot]

Unit tests: pass. 62365 tests passed, 0 failed and 839 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[jpienaar]

mlir/include/mlir/Support/MlirOpt.h

1

s/main/entry point/ (especially if we remove other main references).

25

This was chosen this way to match TableGenMain entry point specification.

My preference would be to just update the CMake files without renaming the file or function: this feels stylistic with no real increase in clarity but would require updating all callers in dependent project. That way you get the consistency between cmake and bazel without further changes.

[mehdi_amini]

mlir/tools/mlir-opt/CMakeLists.txt

15

Why is this a library? It seems not used right now.

(I see it is passed in the link of mlir-opt below, but that seems like a no-op since the only source file for the library is also passed directly as an object to this target)

[marbre]

mlir/include/mlir/Support/MlirOpt.h

25

Thanks for your review. Prior to sending a new diff (which excludes renaming of files and the function), what target names would you propose? Just swapping MLIROptMain and MLIRMlirOptLib seems to break the naming convention. I see the following possibilities:

1. possibility (as proposed so far):
   • MLIROptMain -> MLIROpt
   • MLIRMlirOptLib -> MLIRMlirOpt
2. possibility
   • MLIROptMain -> MLIROptLib
   • MLIRMlirOptLib -> MLIRMlirOptMain

The second would match TensorFlow's Bazel configuration closest, but to me it doesn't feel quite so clear. Looking forward for your feedback.

mlir/tools/mlir-opt/CMakeLists.txt

15

I think the dep of mlir-opt on MLIROptMain could be dropped. So having MLIROptMain as a library would not be necessary here, but it's nice for out-of-tree projects, like IREE.

[marbre]

I went for the second option to get close to TensorFlow's Bazel configuration.

[jpienaar]

This gets us closer to matching the bazel side and works for me, thanks

[marbre]

Thanks for the review. I would appreciate if someone could commit the patch on my behalf.

[mehdi_amini]

Can you rebase? The patch does not apply

[marbre]

Rebased onto 6bfc45cf6. The patch should apply now.

[marbre]

Gentle ping. I would be happy if someone could commit the patch on my behalf.

[marbre]

Thanks!

mlir/include/mlir/Support/MlirOpt.h

-//===- MlirOptMain.h - MLIR Optimizer Driver main ---------------*- C++ -*-===//
+//===- MlirOpt.h - MLIR Optimizer Driver main -------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Main entry function for mlir-opt for when built as standalone binary.
 //
 //===----------------------------------------------------------------------===//
 
 #include <memory>
 #include <vector>
 
 namespace llvm {
 class raw_ostream;
 class MemoryBuffer;
 } // end namespace llvm
 
 namespace mlir {
 struct LogicalResult;
 class PassPipelineCLParser;
 
-LogicalResult MlirOptMain(llvm::raw_ostream &os,
-                          std::unique_ptr<llvm::MemoryBuffer> buffer,
-                          const PassPipelineCLParser &passPipeline,
-                          bool splitInputFile, bool verifyDiagnostics,
-                          bool verifyPasses);
+LogicalResult MlirOpt(llvm::raw_ostream &os,
 
 } // end namespace mlir

mlir/lib/Support/CMakeLists.txt

 set(LLVM_OPTIONAL_SOURCES
   FileUtilities.cpp
   JitRunner.cpp
-  MlirOptMain.cpp
+  MlirOpt.cpp
   StorageUniquer.cpp
   ToolUtilities.cpp
   TranslateClParser.cpp
 )
 
 add_llvm_library(MLIRSupport
   FileUtilities.cpp
   StorageUniquer.cpp
   ToolUtilities.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Support
   )
 target_link_libraries(MLIRSupport LLVMSupport ${LLVM_PTHREAD_LIB})
 
-add_llvm_library(MLIROptMain
-  MlirOptMain.cpp
+add_llvm_library(MLIROpt
+  MlirOpt.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Support
   )
-target_link_libraries(MLIROptMain
+target_link_libraries(MLIROpt
   MLIRPass
   LLVMSupport
   MLIRSupport
   )
 
 add_llvm_library(MLIRTranslateClParser
   TranslateClParser.cpp
 

mlir/lib/Support/MlirOpt.cpp

-//===- MlirOptMain.cpp - MLIR Optimizer Driver ----------------------------===//
+//===- MlirOpt.cpp - MLIR Optimizer Driver --------------------------------===//
 //
 // 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 is a utility that runs an optimization pass and prints the result back
 // out. It is designed to support unit testing.
 //
 //===----------------------------------------------------------------------===//
 
-#include "mlir/Support/MlirOptMain.h"
+#include "mlir/Support/MlirOpt.h"
 #include "mlir/Analysis/Passes.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/Location.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/IR/Module.h"
 #include "mlir/Parser.h"
 #include "mlir/Pass/Pass.h"
static LogicalResult processBuffer(raw_ostream &os,
   performActions(os, verifyDiagnostics, verifyPasses, sourceMgr, &context,
                  passPipeline);
 
   // Verify the diagnostic handler to make sure that each of the diagnostics
   // matched.
   return sourceMgrHandler.verify();
 }
 
-LogicalResult mlir::MlirOptMain(raw_ostream &os,
-                                std::unique_ptr<MemoryBuffer> buffer,
-                                const PassPipelineCLParser &passPipeline,
-                                bool splitInputFile, bool verifyDiagnostics,
-                                bool verifyPasses) {
+LogicalResult mlir::MlirOpt(raw_ostream &os,
   // The split-input-file mode is a very specific mode that slices the file
   // up into small pieces and checks each independently.
   if (splitInputFile)
     return splitAndProcessBuffer(
         std::move(buffer),
         [&](std::unique_ptr<MemoryBuffer> chunkBuffer, raw_ostream &os) {
           return processBuffer(os, std::move(chunkBuffer), verifyDiagnostics,
                                verifyPasses, passPipeline);
         },
         os);
 
   return processBuffer(os, std::move(buffer), verifyDiagnostics, verifyPasses,
                        passPipeline);
 }

mlir/tools/mlir-opt/CMakeLists.txt

 set(LLVM_OPTIONAL_SOURCES
   null.cpp
 )
 
 set(LIB_LIBS
   MLIRAnalysis
   MLIRLLVMIR
   MLIRParser
   MLIRPass
   MLIRTransforms
   MLIRSupport
 )
-add_llvm_library(MLIRMlirOptLib
+add_llvm_library(MLIRMlirOpt
   mlir-opt.cpp
 )
-target_link_libraries(MLIRMlirOptLib ${LIB_LIBS})
+target_link_libraries(MLIRMlirOpt ${LIB_LIBS})
 
 set(LIBS
   MLIRAnalysis
   MLIRAffineOps
   MLIRAffineToStandard
   MLIRLoopsToGPU
   MLIRLinalgToLLVM
 
   MLIRLoopToStandard
   MLIREDSC
   MLIRFxpMathOps
   MLIRGPU
   MLIRGPUtoNVVMTransforms
   MLIRGPUtoROCDLTransforms
   MLIRGPUtoSPIRVTransforms
   MLIRLinalgOps
   MLIRLLVMIR
   MLIRLoopOps
   MLIRNVVMIR
   MLIROpenMP
-  MLIROptMain
+  MLIROpt
   MLIRParser
   MLIRPass
   MLIRQuantizerTransforms
   MLIRQuantOps
   MLIRROCDLIR
   MLIRSPIRV
   MLIRStandardToSPIRVTransforms
   MLIRSPIRVTestPasses
list(APPEND LIBS
     MLIRGPUtoCUDATransforms
   )
 endif()
 add_llvm_tool(mlir-opt
  mlir-opt.cpp
 )
 llvm_update_compile_flags(mlir-opt)
 whole_archive_link(mlir-opt ${LIBS})
-target_link_libraries(mlir-opt PRIVATE MLIRIR MLIRMlirOptLib ${LIBS} LLVMSupport)
+target_link_libraries(mlir-opt PRIVATE MLIRIR MLIRMlirOpt ${LIBS} LLVMSupport)

mlir/tools/mlir-opt/mlir-opt.cpp

 //===- mlir-opt.cpp - MLIR Optimizer Driver -------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // Main entry function for mlir-opt for when built as standalone binary.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/Analysis/Passes.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassManager.h"
 #include "mlir/Support/FileUtilities.h"
-#include "mlir/Support/MlirOptMain.h"
+#include "mlir/Support/MlirOpt.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/ToolOutputFile.h"
 
 using namespace llvm;
 using namespace mlir;
 
int main(int argc, char **argv) {
   }
 
   auto output = openOutputFile(outputFilename, &errorMessage);
   if (!output) {
     llvm::errs() << errorMessage << "\n";
     exit(1);
   }
 
-  return failed(MlirOptMain(output->os(), std::move(file), passPipeline,
-                            splitInputFile, verifyDiagnostics, verifyPasses));
+  return failed(MlirOpt(output->os(), std::move(file), passPipeline,
 }