Add mlir graph optimization to the build #39231
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joker-eph
added
kokoro:force-run
|
Thanks for the fix! Seems like the CI is not yet happy with it right now. |
yongtang
added a commit
to yongtang/tensorflow
that referenced
this pull request
May 7, 2020
…gistration" to tensorflow/python/_pywrap_mlir.so This commit adds build target //tensorflow/compiler/mlir/tensorflow:graph_optimization_pass_registration to tensorflow/python/_pywrap_mlir.so, so that graph optimization could be available in tf-nightly pip wheel. In the past while graph_optimization_pass_registration is available, it is not packaged with tf-nightly so it is not possible to access graph optimization if installed from pip wheel. In order to enable graph optimization pass for pip wheel, graph_optimization_pass_registration target has to be included in "somewhere" that will be loaded when pip installed tensorflow is imported. A natural place would be libtensorflow_framework.so which is the core .so always loaded with `import tensorflow`. It is possible to include graph_optimization_pass_registration to target libtensorflow_framework.so, see last attempt on this route: tensorflow#39231 However, this caused many test failures like: ``` : CommandLine Error: Option 'help-list' registered more than once! LLVM ERROR: inconsistency in registered CommandLine options ``` The reason is that many tests as a binary also have a copy of the LLVM in its binary, thus causing multiple copies (one in binary, another one in libtensorflow_framework.so where many tests depends on). Because there are so many tests, it is really hard to make all the needed changes without break somewhere else. This commit takes a different approach to include graph_optimization_pass_registration to tensorflow/python/_pywrap_mlir.so. This shared object is dedicated to mlir related APIs. The current exposed one is `tf.mlir.experimental.convert_graph_def`. Because this tensorflow/python/_pywrap_mlir.so already depends on llvm, place graph_optimization_pass will avoid multiple copies of llvm in multiple locations. This tensorflow/python/_pywrap_mlir.so is also loaded with `import tensorflow` as part of the python binding. Ideally it probably would be still preferrale to get the graph_optimization_pass into libtensorflow_framework.so. That will be investigated further later. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
…gistration" to tensorflow/python/_pywrap_mlir.so This commit adds build target //tensorflow/compiler/mlir/tensorflow:graph_optimization_pass_registration to tensorflow/python/_pywrap_mlir.so, so that graph optimization could be available in tf-nightly pip wheel. In the past while graph_optimization_pass_registration is available, it is not packaged with tf-nightly so it is not possible to access graph optimization if installed from pip wheel. In order to enable graph optimization pass for pip wheel, graph_optimization_pass_registration target has to be included in "somewhere" that will be loaded when pip installed tensorflow is imported. A natural place would be libtensorflow_framework.so which is the core .so always loaded with `import tensorflow`. It is possible to include graph_optimization_pass_registration to target libtensorflow_framework.so, see last attempt on this route: tensorflow#39231 However, this caused many test failures like: ``` : CommandLine Error: Option 'help-list' registered more than once! LLVM ERROR: inconsistency in registered CommandLine options ``` The reason is that many tests as a binary also have a copy of the LLVM in its binary, thus causing multiple copies (one in binary, another one in libtensorflow_framework.so where many tests depends on). Because there are so many tests, it is really hard to make all the needed changes without break somewhere else. This commit takes a different approach to include graph_optimization_pass_registration to tensorflow/python/_pywrap_mlir.so. This shared object is dedicated to mlir related APIs. The current exposed one is `tf.mlir.experimental.convert_graph_def`. Because this tensorflow/python/_pywrap_mlir.so already depends on llvm, place graph_optimization_pass will avoid multiple copies of llvm in multiple locations. This tensorflow/python/_pywrap_mlir.so is also loaded with `import tensorflow` as part of the python binding. Ideally it probably would be still preferrale to get the graph_optimization_pass into libtensorflow_framework.so. That will be investigated further later. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
|
Thanks @joker-eph for the review. The tests error was due to tests normally depends on LLVM so there are two copies again (one in libtensorflow_io.so and one in tests binary itself). While it is possible to updates the tests, there are so many and dependencies are just too interleaved. I take another look and now the PR takes a different approach: it adds
Since In the long term it still might makes sense to build |
|
@joker-eph The PR has been updated. The |
joker-eph
approved these changes
May 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is part of the process to resolve #39135.
As was mentioned in #39135, 248bc00 enables MLIR graph optimizations. However, it is not part of the build for pip wheel. The reason was that MLIR graph optimizations is done through a registration (mlir_graph_optimization_pass_registration.cc) but this file is not included in
libtensorflow_framerowk.soso pip wheel package is not enabled.This PR add the
mlir_graph_optimization_pass_registrationto be part of thelibtensorflow_framework.so. Due to thebazeldependency reasons, a direct inclusion will not work as ops in core and xla will be pulled in multiple times by bazel. There will be 2 copies of xla ops, 2 copies of lite and core ops inlibtensorflow_framework.soSo the following change has been made. In general
import_model.[h|cc]has been split into 3 parts:import_base.[h|cc] consists of ImporterBase class and common shared functions (byimport_model.[h|cc]andimport_graphdef.[h|cc]`import_graphdef.[h|cc]consists of graph only conversion to mlirimport_model.[h|cc]has also been updated to consist model only conversion to mlirThere are also some small bazel changes that removes unnecessary dependencies.
Note with the change, MLIR graph optimizations only need to depends on
import_graphdef.ccandexport_graphdef.cc. It will not depend onimport_model.ccwhich pulled in xla and core ops multiple time by bazel.Signed-off-by: Yong Tang yong.tang.github@outlook.com