Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[r2.2:Cherryppick] Split unroll_batch_matmul pass into own build target #37362

Merged
merged 4 commits into from Mar 10, 2020

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Mar 5, 2020

This is a cherry-pick of #37356

@jpienaar Since this doesn't include functional changes it would be amazing if it could be cherry-picked onto the 2.2 release branch.

mihaimaruseac
mihaimaruseac previously approved these changes Mar 5, 2020
@mihaimaruseac
Copy link
Collaborator

mihaimaruseac commented Mar 9, 2020

I think you are missing the removal of these lines

#include "mlir/Dialect/QuantOps/FakeQuantSupport.h" // TF:llvm-project
#include "mlir/Dialect/QuantOps/UniformSupport.h" // TF:llvm-project

It was in #37356 but it is not here. Is it possible the cherrypick was wrong?

@lgeiger
Copy link
Contributor Author

lgeiger commented Mar 9, 2020

@mihaimaruseac If I compare 0e86d51 and this PR side by side the only difference are the changes in tensorflow/compiler/mlir/tensorflow/transforms/batchmatmul_to_einsum.cc which are not relevant on the r2.2 branch since the file doesn't exist, or am I missing something?

@mihaimaruseac
Copy link
Collaborator

Oh, sorry, I was looking at the wrong file.

However, there are still differences between the PR on master and the PR on this branch: On master https://github.com/tensorflow/tensorflow/pull/37356/files#diff-f20d136d02b72175cd9891df563f8d62R267-R280. On this branch https://github.com/tensorflow/tensorflow/pull/37362/files#diff-f20d136d02b72175cd9891df563f8d62R267-R287 There are more deps added on this branch than on master.

@lgeiger
Copy link
Contributor Author

lgeiger commented Mar 9, 2020

However, there are still differences between the PR on master and the PR on this branch

This is intentional since during the merge of #37356 those dependencies where added to resolve internal build errors mentioned in #37356 (comment)

See 0e86d51 for the diff of the merged PR into master.

@mihaimaruseac
Copy link
Collaborator

Makes sense. Thank you

@goldiegadde goldiegadde added the kokoro:force-run Tests on submitted change label Mar 10, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Mar 10, 2020
@goldiegadde goldiegadde merged commit 1aaea1f into tensorflow:r2.2 Mar 10, 2020
@lgeiger lgeiger deleted the r2.2-split-unroll-bn-pass branch March 10, 2020 01:40
lgeiger added a commit to larq/compute-engine that referenced this pull request Mar 10, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Mar 11, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Mar 19, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Apr 1, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Apr 1, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Apr 8, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Apr 8, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request Apr 22, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
lgeiger added a commit to larq/compute-engine that referenced this pull request May 4, 2020
This includes tensorflow/tensorflow#37362 which should reduce the amount of dependencies to build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants