Add unboxedCallRedispatch#35476
Conversation
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> [ghstack-poisoned]
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 250a4ed Pull Request resolved: #35476
💊 CircleCI build failures summary and remediationsAs of commit ae3547c (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no CircleCI failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker. This comment has been revised 62 times. |
smessmer
left a comment
There was a problem hiding this comment.
Why do we introduce a new BackendGeneric instead of just reusing BackendSelect and potentially renaming it?
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
Because... they have different semantic meanings? |
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
They're different use cases but they're the same mechanism of "do some work before calling into the actual kernel". I was thinking that maybe we can unify them. |
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
A few things: - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
|
@smessmer I updated this diff to not include BackendGeneric anymore. |
A few things: - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
| template<class Return, class... Args> | ||
| Return callUnboxedWithDispatchKey(const OperatorHandle& op, DispatchKey dispatchKey, Args... args) const; | ||
|
|
||
| // Like callUnboxed, but intended for use in a redispatch: you currently |
| @@ -34,6 +34,7 @@ namespace c10 { | |||
| class DispatchKeySet final { | |||
| public: | |||
| enum Full { FULL }; | |||
There was a problem hiding this comment.
Thinking about it, I probably would have put those into static factory functions instead of constructors with an enum argument, i.e.
static DispatchKeySet DispatchKeySet::full();
static DispatchKeySet DispatchKeySet::raw();
static DispatchKeySet DispatchKeySet::full_after(...);
Seems more standard. But not a strong opinion if you wanna keep it as is.
There was a problem hiding this comment.
I'm in the habit of doing constructors because it makes field initialization syntax better (doesn't matter here as we have copy/move constructors but when you don't have those it makes a ton of difference.)
A few things: - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: [D20680518](https://our.internmc.facebook.com/intern/diff/D20680518) [ghstack-poisoned]
A few things: - Add a new BackendGeneric dispatch key, intended to be a place where we can put code which is generic across backends. No operators are modified to use it but there is a simple use case test in op_registration_test.cpp. It's enabled by default (like Variable) but fallthrough by default (like BackendSelect) - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> ghstack-source-id: 53143d0 Pull Request resolved: pytorch/pytorch#35476
Summary: Pull Request resolved: pytorch#35476 A few things: - Add new callUnboxedRedispatch function which can be used to do a redispatch when you don't want to add a type id to the excluded set. This will recompute the dispatch key but ignore everything including and before the currentDispatchKey - Add FULL_AFTER constructor to DispatchKeySet; used to implement redispatch. Signed-off-by: Edward Z. Yang <ezyang@fb.com> Differential Revision: D20680518 Test Plan: Imported from OSS Pulled By: ezyang fbshipit-source-id: ecd7fbdfa916d0d2550a5b19dd3ee4a9f2272457
Stack from ghstack:
A few things:
redispatch when you don't want to add a type id to the excluded
set. This will recompute the dispatch key but ignore everything
including and before the currentDispatchKey
redispatch.
Signed-off-by: Edward Z. Yang ezyang@fb.com
Differential Revision: D20680518