-
Notifications
You must be signed in to change notification settings - Fork 39
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
Miopen dialect opt step19 : rename attributes for miopen.transform
#243
Merged
+1,022
−890
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
ce09f5c
to
67c4ef1
Compare
miopen.transform
ca331e3
to
391ca2b
Compare
domain -> output_bounds range -> source_bounds
To help users easily understand the relationship between different transformations, apply the following naming changes to attributes used in miopen.transform: source_dimensions -> lower_layer_dimensions dimensions -> upper_layer_dimensions source_names -> lower_layer_names names -> upper_layer_names source_bounds -> lower_layer_bounds output_bounds -> upper_layer_bounds Upper layer is more abstract which has more coordinate transformations applied. Lower layer has less coordinate transformations applied. To tame C++ code production path, a "lowest_layer" attribute is introduced.
391ca2b
to
f1d782c
Compare
jenkins: retest this please. |
jenkins: retest this please. |
asleepzzz
approved these changes
May 31, 2021
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.
@asroy This PR gives you an idea the scope of renaming attributes. And it also gives you an idea that's it's better to have a declarative way to specify coordinate transformations in TableGen, not in C++.
Rename attributes used in
miopen.transform
. Addlowest_layer
attribute.To help users easily understand the relationship between different
transformations, apply the following naming changes to attributes used
in
miopen.transform
:source_layout
->lower_layer_layout
intermeidate_layout
->lower_layer_layout
output_layout
->upper_layer_layout
source_dimensions
->lower_layer_dimensions
dimensions
->upper_layer_dimensions
source_names
->lower_layer_names
names
->upper_layer_names
source_bounds
->lower_layer_bounds
output_bounds
->upper_layer_bounds
domain
->lower_layer_bounds
range
->upper_layer_bounds
Upper layer is more abstract which has more coordinate transformations
applied. Lower layer has less coordinate transformations applied.
To tame C++ code production path, a
lowest_layer
attribute isintroduced.
@jerryyin because this PR touches the C++ code generation path so I also invited you to review this PR as there are non-trivial changes to C++ code generation path.
Originally, we rely on the difference between
source_layout
andintermediate_layout
but now there is nointermediate_layout
anymore. Everything has been renamed tolower_layer_layout
. I have to introduce a new attributelowest_layer
to help C++ code generation path to figure out the lowest level and do appropriate C++ code emission.This PR is not blocking #226 , although it is indeed better to have it merged before I revise #226 so we have a consistent attribute naming convention across all kinds of coordinate transformations.