Add overload name to JIT prim operators. #29656
Conversation
facebook-github-bot
added
the
oncall: jit
| []() { | ||
| }) | ||
| ).op( | ||
| "_aten::append.Tensor(Tensor self) -> void", |
There was a problem hiding this comment.
The schema here do not match the implementation of the operator. If this is untested in the mobile interpreter, then many of the schema in this file may be wrong.
There was a problem hiding this comment.
Yep, schema in this file may be wrong. @smessmer Just curious: if we are using names only, would the schema be necessary in mobile? It looks like schema is required for boxed registration, but it's not checked in the registration itself. Could you confirm if a correct schema is required for registrations in this file. If it is, how to know what's the correct schema?
There was a problem hiding this comment.
if you register multiple ops with the same operator name + overload name but mismatching schemas, the operator library will throw an error. This didn't happen here because _aten::append.Tensor is allowed to mismatch with aten::append.Tensor, they're different ops.
Differential Revision: [D18499600](https://our.internmc.facebook.com/intern/diff/D18499600) [ghstack-poisoned]
Differential Revision: [D18499600](https://our.internmc.facebook.com/intern/diff/D18499600) [ghstack-poisoned]
Differential Revision: [D18499600](https://our.internmc.facebook.com/intern/diff/D18499600) [ghstack-poisoned]
Differential Revision: [D18499600](https://our.internmc.facebook.com/intern/diff/D18499600) [ghstack-poisoned]
Differential Revision: [D18499600](https://our.internmc.facebook.com/intern/diff/D18499600) [ghstack-poisoned]
|
This is causing a test failure on master, I've started the unland to fix: |
|
This pull request has been merged in ff4e782. |
Overload name is required in mobile operators with the same name but different schema. Since it's not used in JIT, it's safe to add overload names for JIT operators. Re-commit after a revert of #29656, because of backward compatibility failure. Adding overload name also triggers backward compatibility failures, so the overload names of append are added in this PR. Will contact the backward compatibility to add more overload names. Differential Revision: [D18555484](https://our.internmc.facebook.com/intern/diff/D18555484)
5 participants
Stack from ghstack:
Differential Revision: D18499600