[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 ↗

(On Diff #241701)

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

25 ↗

(On Diff #241701)

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

16

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 ↗

(On Diff #241701)

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

16

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/lib/Support/CMakeLists.txt

add_llvm_library(MLIRSupport
   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
+add_llvm_library(MLIROptLib
   MlirOptMain.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Support
   )
-target_link_libraries(MLIROptMain
+target_link_libraries(MLIROptLib
   MLIRPass
   MLIRParser
   LLVMSupport
   MLIRSupport
   )
 
 add_llvm_library(MLIRTranslateClParser
   TranslateClParser.cpp

mlir/tools/mlir-opt/CMakeLists.txt

 set(LLVM_OPTIONAL_SOURCES
   null.cpp
 )
 
 set(LIB_LIBS
   MLIRAnalysis
   MLIRLLVMIR
-  MLIROptMain
+  MLIROptLib
   MLIRParser
   MLIRPass
   MLIRTransforms
   MLIRSupport
 )
-add_llvm_library(MLIRMlirOptLib
+add_llvm_library(MLIRMlirOptMain
   mlir-opt.cpp
 )
-target_link_libraries(MLIRMlirOptLib
+target_link_libraries(MLIRMlirOptMain
   ${LIB_LIBS}
 )
 
 set(FULL_LINK_LIBS
   MLIRLoopAnalysis
   MLIRAnalysis
   MLIRAffineOps
   MLIRAffineToStandard
set(FULL_LINK_LIBS
   MLIRLinalgAnalysis
   MLIRLinalgEDSC
   MLIRLinalgTransforms
   MLIRLinalgUtils
   MLIRLLVMIR
   MLIRLoopOps
   MLIRNVVMIR
   MLIROpenMP
-  MLIROptMain
+  MLIROptLib
   MLIRParser
   MLIRPass
   MLIRQuantizerFxpMathConfig
   MLIRQuantizerSupport
   MLIRQuantizerTransforms
   MLIRQuantOps
   MLIRROCDLIR
   MLIRSPIRV
set(FULL_LINK_LIBS
   MLIRTestTransforms
   MLIRSupport
   MLIRVectorOps
   MLIRVectorToLLVM
   MLIRVectorToLoops
 )
 set(LIBS
   MLIRIR
-  MLIROptMain
+  MLIROptLib
   LLVMSupport
   LLVMCore
   LLVMAsmParser
 )
 
 if(MLIR_CUDA_CONVERSIONS_ENABLED)
   list(APPEND LIBS
     LLVMNVPTXCodeGen