Add layout transformer for NNAPI #10371
Merged
Conversation
This file contains hidden or 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
askhade
commented
Jan 25, 2022
skottmckay
reviewed
Jan 27, 2022
onnxruntime/core/optimizer/transpose_optimizer/transpose_optimizer.cc
Outdated
Show resolved
Hide resolved
skottmckay
reviewed
Feb 1, 2022
skottmckay
reviewed
Feb 1, 2022
skottmckay
reviewed
Feb 4, 2022
onnxruntime/core/providers/nnapi/nnapi_builtin/builders/op_builder.cc
Outdated
Show resolved
Hide resolved
edgchen1
reviewed
Feb 4, 2022
| bool operator()(const Node* n1, const Node* n2) const; | ||
| }; | ||
|
|
||
| enum class DataLayout { |
Contributor
There was a problem hiding this comment.
why is DataLayout defined in graph_viewer.h?
maybe move it to its own file so it can easily be included where needed?
edgchen1
reviewed
Feb 4, 2022
edgchen1
reviewed
Feb 4, 2022
edgchen1
reviewed
Feb 4, 2022
edgchen1
reviewed
Feb 4, 2022
edgchen1
reviewed
Feb 5, 2022
edgchen1
reviewed
Feb 5, 2022
edgchen1
reviewed
Feb 5, 2022
|
|
||
| // initialize in-degrees and find root nodes | ||
| for (const auto& node : graph_viewer.Nodes()) { | ||
| for (const auto& node_index : graph_viewer.GetNodesInTopologicalOrder()) { |
Contributor
Contributor
Author
There was a problem hiding this comment.
Changed it back ... I added it when I was debugging an old issue...
edgchen1
reviewed
Feb 5, 2022
askhade
changed the title
edgchen1
reviewed
Feb 9, 2022
edgchen1
reviewed
Feb 9, 2022
edgchen1
reviewed
Feb 9, 2022
| // Computes permutation from channel last to channel first ordering of given rank. Nearly all handlers work for any | ||
| // permutation, but some are restricted. Also used for layout transformation. Rank must be >= 1. | ||
| std::vector<int64_t> ChannelLastToFirstPerm(size_t rank) { | ||
| if (rank == 0) { |
Contributor
Contributor
Author
There was a problem hiding this comment.
api.h says rank must be >=2... updated the condition
edgchen1
reviewed
Feb 9, 2022
onnxruntime/core/optimizer/transpose_optimizer/transpose_optimizer.cc
Outdated
Show resolved
Hide resolved
edgchen1
reviewed
Feb 9, 2022
edgchen1
reviewed
Feb 9, 2022
| // Please update the following 3 places: | ||
| // 1. api_impl.cc "onnx_ops_available_versions" map, include the latest version in the map | ||
| // 2. kernel_registry_manager.cc "static_kernel_hashes" include an entry for latest version and it's associated hash | ||
| // 3. This file "onnx_ops_available_versions" map, include the latest version in the map |
Contributor
Contributor
Author
There was a problem hiding this comment.
I thought of it... From this test perspective exposing this map via a Get mtd makes sense, however since the usage of this map is internal to api::AddNode I did not take that route.
skottmckay
reviewed
Feb 10, 2022
| * @return std::optional<HashValue> | ||
| */ | ||
| std::optional<HashValue> GetHashValueFromStaticKernelHashMap(const std::string& op_type, int since_version); | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
nit: can we make the relationship between the transpose optimizer a little clearer as standalone 'static kernel def hashes map' in the framework library is very general. maybe putting it in the onnxruntime::transpose_optimizer namespace would help.
'sesison state' -> 'session state'
skottmckay
reviewed
Feb 10, 2022
skottmckay
reviewed
Feb 15, 2022
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.
Description: Adding layout transformer which leverages the transpose optimizer for NCHW -> NHWC layout conversions for compile-based EPs. In the first phase converting NNAPI to use this.
Motivation and Context