[GPU] Onednn integration for handling reorders#7764
Merged
isanghao merged 1 commit intoopenvinotoolkit:masterfrom Oct 13, 2021
Merged
[GPU] Onednn integration for handling reorders#7764isanghao merged 1 commit intoopenvinotoolkit:masterfrom
isanghao merged 1 commit intoopenvinotoolkit:masterfrom
Conversation
lznamens
suggested changes
Oct 1, 2021
f9c4503 to
c3d2150
Compare
lznamens
approved these changes
Oct 5, 2021
aeb8e1a to
86903fd
Compare
isanghao
reviewed
Oct 6, 2021
|
|
||
| bool all_users_fuse = true; | ||
| std::vector<program_node*> recalc_list; | ||
|
|
Contributor
There was a problem hiding this comment.
I found there is change like below. Did you omit it intentionally?
@@ -318,7 +318,6 @@ void remove_redundant_reorders::run(program_impl& p) {
auto& dep = node_ptr->get_dependency(0);
if (!usr->is_type<quantize>() ||
(dep.get_output_layout().format != format::b_fs_yx_fsv16 &&
- dep.get_output_layout().format != format::fs_b_yx_fsv32 &&
dep.get_output_layout().format != format::bfyx))
continue;
| (usr->get_output_layout().data_type != dep.get_output_layout().data_type) || | ||
| (dep.get_output_layout().format != format::bfyx) || | ||
| (usr->get_output_layout().format != format::fs_b_yx_fsv32)) | ||
| (usr->get_output_layout().format != format::fs_b_yx_fsv32 && usr->get_output_layout().format != format::b_fs_yx_fsv32) || |
Contributor
There was a problem hiding this comment.
This is now mismatching with the comment. Could you check the comment just above this line?
Contributor
There was a problem hiding this comment.
I found that this code was initially inserted by @sshlyapn.
Sergey, do we have update in convolution code as well to support b_fs_yx_fsv32?
Contributor
There was a problem hiding this comment.
conv fusion of (b_fs_yx_fsv32 reorder) is supported only on onednn.
Contributor
Author
There was a problem hiding this comment.
it will be fixed as discussed.
Contributor
There was a problem hiding this comment.
I think this is quite became quite difficult now. What about rewriting as below? It will include the diff in line 366.
if (!(usr->is_type<convolution>()) ||
usr->get_output_layout().data_type != dep.get_output_layout().data_type ||
dep.get_output_layout().format != format::bfyx))
return;
if (usr->as<convolution().get_preferred_impl_type() == impl_types::cldnn &&
usr->get_output_layout().format != format::fs_b_yx_fsv32)
return;
if (usr->as<convolution().get_preferred_impl_type() == impl_types::onednn &&
usr->get_output_layout().format != format::b_fs_yx_fsv32)
return;
1256933 to
bbad62c
Compare
isanghao
reviewed
Oct 8, 2021
| if ((usr->as<quantize>().get_preferred_impl_type() != impl_types::onednn) && | ||
| (dep.get_output_layout().format == format::fs_b_yx_fsv32)) | ||
| continue; | ||
|
|
Contributor
There was a problem hiding this comment.
From what I remember, we should not apply this snippet. 1) fs_b_yx_fsv32 won't be selected for onednn. 2) we should not touch cldnn logic.
Contributor
Author
There was a problem hiding this comment.
Ok it will be removed.
| (usr->get_output_layout().data_type != dep.get_output_layout().data_type) || | ||
| (dep.get_output_layout().format != format::bfyx) || | ||
| (usr->get_output_layout().format != format::fs_b_yx_fsv32)) | ||
| (usr->get_output_layout().format != format::fs_b_yx_fsv32 && usr->get_output_layout().format != format::b_fs_yx_fsv32) || |
Contributor
There was a problem hiding this comment.
I think this is quite became quite difficult now. What about rewriting as below? It will include the diff in line 366.
if (!(usr->is_type<convolution>()) ||
usr->get_output_layout().data_type != dep.get_output_layout().data_type ||
dep.get_output_layout().format != format::bfyx))
return;
if (usr->as<convolution().get_preferred_impl_type() == impl_types::cldnn &&
usr->get_output_layout().format != format::fs_b_yx_fsv32)
return;
if (usr->as<convolution().get_preferred_impl_type() == impl_types::onednn &&
usr->get_output_layout().format != format::b_fs_yx_fsv32)
return;
c0025ab to
909caa9
Compare
Contributor
Author
|
Passed dry-run with gpu_tools/dryrun_benchmark.sh update. |
jade-cho
approved these changes
Oct 12, 2021
isanghao
approved these changes
Oct 12, 2021
Signed-off-by: Min, Byungil <byungil.min@intel.com>
909caa9 to
48cd925
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Signed-off-by: Min, Byungil byungil.min@intel.com
Details:
Tickets: