[GPU] Revised unique ID setting scheme. #10548
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
yeonbok
changed the title
yeonbok
requested review from
ahnyoung-paul,
geunhwan,
vladimir-paramuzov,
kelvinchoi-intel and
andrew-k-park
yeonbok
force-pushed
the
taylor_fix_entry_point
branch
from
9f4c366 to
46f9abb
Compare
| if (myprog.get_parent_primitive() != "") { | ||
| auto loop_prim = myprog.get_parent_primitive(); | ||
| std::replace(loop_prim.begin(), loop_prim.end(), '/', '_'); | ||
| std::replace(loop_prim.begin(), loop_prim.end(), ':', '_'); |
There was a problem hiding this comment.
I think those two are not the only prohibited symbols for kernel name... Is it possible to use processing ID of parent primitive instead?
There was a problem hiding this comment.
There is no processing id -like thing currently, but I was thinking of changing this to be "body_network_id" like thing, how do you think?
I.e.,
- Let the program have body_prog_id (e.g., if the program has 3 loops, ids 0-2 exists for that program)
- when build_body_network, set the body_prog_id
- Use it in its unique_id
There was a problem hiding this comment.
I just decided to use unique_id as a temporally generated number same as the original behavior to make it simple. I tried the above mentioned impl but thought again, there are too many "_id"s .. maybe it will be better to let the program have parent / body impl more structured way. However at this moment it is only needed for distinguish the entry point, no other usage for them. Please take a look..
There was a problem hiding this comment.
So I am gonna revert the changes w.r.t the parent_id things, too, soon
yeonbok
force-pushed
the
taylor_fix_entry_point
branch
from
883d42f to
5d3cf09
Compare
…to distinguish the loop body networks' id. However, it results in cl cache miss for same network loaded multiple time, because program ids are differnt. Now revised it to use parent primitive id instead of program_id for unique id of nodes in body networks.
yeonbok
force-pushed
the
taylor_fix_entry_point
branch
from
5d3cf09 to
4afc08a
Compare
vladimir-paramuzov
approved these changes
Feb 22, 2022
kelvinchoi-intel
pushed a commit
to kelvinchoi-intel/openvino
that referenced
this pull request
Mar 17, 2022
* Revised unique ID setting scheme. Previously it was using program id to distinguish the loop body networks' id. However, it results in cl cache miss for same network loaded multiple time, because program ids are differnt. Now revised it to use parent primitive id instead of program_id for unique id of nodes in body networks. * Revised adding unique_id to entry points to have a temporal number as unique id * Revert the canceld change * Added test to check whether two networks loaded from same function creates same cl cache
yeonbok
added a commit
to yeonbok/openvino
that referenced
this pull request
Mar 17, 2022
* Revised unique ID setting scheme. Previously it was using program id to distinguish the loop body networks' id. However, it results in cl cache miss for same network loaded multiple time, because program ids are differnt. Now revised it to use parent primitive id instead of program_id for unique id of nodes in body networks. * Revised adding unique_id to entry points to have a temporal number as unique id * Revert the canceld change * Added test to check whether two networks loaded from same function creates same cl cache Co-authored-by: Taylor Yeonbok Lee <[email protected]>
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.
Details:
Revised unique ID setting scheme. Previously it was using program id to distinguish the loop body networks' id.
However, it results in cl cache miss for same network loaded multiple time, because program ids are differnt.
Now revised it to use parent primitive id instead of program_id for unique id of nodes in body networks.
Tickets: