use in-tree public codegen API. by bdhirsh · Pull Request #2898 · pytorch/xla

bdhirsh · 2021-04-19T18:55:18Z

This PR updates the build process to use in-tree pytorch codegen, which is getting merged here: pytorch/pytorch#56601 (more details on the codegen are in that PR). Doing so requires a new yaml file, xla_native_functions.yaml.

I also updated the docs to reflect the codegen change. I'm going to save deleting the existing codegen logic for a future PR

ailzhang

LGTM! Thanks @bdhirsh !

JackCaoG · 2021-04-20T19:05:58Z

@bdhirsh I have a question regarding ops that has different signatures. for example in #2900, clamp has two different signatures that takes min and max as scalar or tensor. If I only want to lower one of the variants, how should I do that?

JackCaoG · 2021-04-20T19:06:45Z

Could you also update https://github.com/pytorch/xla/blob/master/OP_LOWERING_GUIDE.md to reflect this new yaml file?

bdhirsh · 2021-04-21T18:41:43Z

@JackCaoG

have a question regarding ops that has different signatures. for example in #2900, clamp has two different signatures that takes min and max as scalar or tensor. If I only want to lower one of the variants, how should I do that?

The operator names come from the in-tree file native_functions.yaml, and follow a convention of being named as <name>.<overload_name> when overloads are required, in order to make them unique without needing schema. In the example you linked with clamp, there's a corresponding change in native_functions.yaml that adds the new clamp overload here (the new operator there is named clamp.Tensor).

In general you can look at native_functions.yaml in-tree to see what the full names of the operators are, when you need to figure out the name of the operator to add the the yaml. There are already a bunch in the xla_native_functions.yaml file included in this PR (just search for .out or .Tensor in that file).

I'll also make sure that's all in the documentation!

Could you also update https://github.com/pytorch/xla/blob/master/OP_LOWERING_GUIDE.md to reflect this new yaml file?

Yep! I'll let you know when I've updated it so you can take a look at the changes

bdhirsh · 2021-04-21T19:19:54Z

updated the docs. I'll need to update the docs more in a later PR, e.g. when I change the file names.

I also unpinned this PR from my feature branch, so CI will keep failing until my other PR merges to master

JackCaoG

Thanks @bdhirsh ! This looks great. Could you wait for #2891 to land first (I chatted with @mruberry and the upstream pytorch pr should merge pretty soon) and update your pr?

I can take care of the rest of the pending op lowering pr and rebase those prs.

I fixed `RegistrationDeclarations.yaml` in a previous PR to account for codegen'd composite kernels, but I forgot to make the corresponding change in the new external codegen. This change needs to land before pytorch/xla#2898. I also moved the function out of `gen.py`, since RegistrationDeclarations.yaml isn't really the main use case for the function anymore- we want to kill it eventually. I also just tried leaving the function in `gen.py` initially and calling `from tools.codegen.gen import has_autogenerated_composite_kernel`. But python didn't like that, and I couldn't figure out why. As a quick attempt to debug, I printed `import tools.codegen; dir(tools.codegen)`, and for some reason `gen` wasn't showing up in the list, even though other subfolders were. This doesn't matter too much though, since I think moving the function out of `gen.py` is the right move anyway. Differential Revision: [D28012667](https://our.internmc.facebook.com/intern/diff/D28012667) [ghstack-poisoned]

…omposite kernels" I fixed `RegistrationDeclarations.yaml` in a previous PR to account for codegen'd composite kernels, but I forgot to make the corresponding change in the new external codegen. This change needs to land before pytorch/xla#2898. I also moved the function out of `gen.py`, since RegistrationDeclarations.yaml isn't really the main use case for the function anymore- we want to kill it eventually. I also just tried leaving the function in `gen.py` initially and calling `from tools.codegen.gen import has_autogenerated_composite_kernel`. But python didn't like that, and I couldn't figure out why. As a quick attempt to debug, I printed `import tools.codegen; dir(tools.codegen)`, and for some reason `gen` wasn't showing up in the list, even though other subfolders were. This doesn't matter too much though, since I think moving the function out of `gen.py` is the right move anyway. Differential Revision: [D28012667](https://our.internmc.facebook.com/intern/diff/D28012667) [ghstack-poisoned]

I fixed `RegistrationDeclarations.yaml` in a previous PR to account for codegen'd composite kernels, but I forgot to make the corresponding change in the new external codegen. This change needs to land before pytorch/xla#2898. I also moved the function out of `gen.py`, since RegistrationDeclarations.yaml isn't really the main use case for the function anymore- we want to kill it eventually. I also just tried leaving the function in `gen.py` initially and calling `from tools.codegen.gen import has_autogenerated_composite_kernel`. But python didn't like that, and I couldn't figure out why. As a quick attempt to debug, I printed `import tools.codegen; dir(tools.codegen)`, and for some reason `gen` wasn't showing up in the list, even though other subfolders were. This doesn't matter too much though, since I think moving the function out of `gen.py` is the right move anyway. Differential Revision: [D28012667](https://our.internmc.facebook.com/intern/diff/D28012667) [ghstack-poisoned]

…orporate #2891

This reverts commit 32fe908.

This reverts commit b1bfffa.

bdhirsh force-pushed the public_codegen_api2 branch from 133a488 to 0dd4eee Compare April 19, 2021 20:41

ailzhang approved these changes Apr 20, 2021

View reviewed changes

JackCaoG approved these changes Apr 21, 2021

View reviewed changes

bdhirsh force-pushed the public_codegen_api2 branch from afa237f to 3addce5 Compare April 23, 2021 22:50

bdhirsh mentioned this pull request Apr 26, 2021

make external codegen aware of autogen'd composite kernels pytorch/pytorch#56960

Closed

bdhirsh force-pushed the public_codegen_api2 branch from 3addce5 to 664bee1 Compare May 3, 2021 17:48

bdhirsh added 9 commits May 10, 2021 04:58

use in-tree public codegen API.

c70b792

use in-tree public codegen API.

be4fa9f

use in-tree public codegen API.

c4ba8c7

use in-tree public codegen API.

eabb13b

updated the docs to mention new codegen

df65320

stop pinning to feature branch

4da205d

remove torch_pin file before merging. preemptively update yaml to inc…

9da14d0

…orporate #2891

update list of ops to be in sync with master

d542016

add clamp ops to xla_native_functions.yaml

2639e8d

bdhirsh force-pushed the public_codegen_api2 branch from 664bee1 to 2639e8d Compare May 10, 2021 12:08

bdhirsh merged commit 32fe908 into master May 10, 2021

bdhirsh deleted the public_codegen_api2 branch May 10, 2021 14:42

bdhirsh added a commit that referenced this pull request May 10, 2021

Revert "use in-tree public codegen API. (#2898)"

7734849

This reverts commit 32fe908.

bdhirsh mentioned this pull request May 10, 2021

Revert "use in-tree public codegen API. (#2898)" #2938

Merged

bdhirsh added a commit that referenced this pull request May 10, 2021

Revert "use in-tree public codegen API. (#2898)" (#2938)

b1bfffa

This reverts commit 32fe908.

bdhirsh restored the public_codegen_api2 branch May 10, 2021 20:18

bdhirsh added a commit that referenced this pull request May 11, 2021

Re-apply the commit: "use in-tree public codegen API. (#2898)"

dfc253d

This reverts commit b1bfffa.

bdhirsh mentioned this pull request May 11, 2021

Re-apply the commit: "use in-tree public codegen API. (#2898)" #2940

Merged

bdhirsh added a commit that referenced this pull request May 11, 2021

Re-apply the commit: "use in-tree public codegen API. (#2898)" (#2940)

9cc9ad9

This reverts commit b1bfffa.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use in-tree public codegen API.#2898

use in-tree public codegen API.#2898
bdhirsh merged 9 commits intomasterfrom
public_codegen_api2

bdhirsh commented Apr 19, 2021 •

edited

Loading

Uh oh!

ailzhang left a comment

Uh oh!

JackCaoG commented Apr 20, 2021

Uh oh!

JackCaoG commented Apr 20, 2021

Uh oh!

bdhirsh commented Apr 21, 2021 •

edited

Loading

Uh oh!

bdhirsh commented Apr 21, 2021 •

edited

Loading

Uh oh!

JackCaoG left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

bdhirsh commented Apr 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ailzhang left a comment

Choose a reason for hiding this comment

Uh oh!

JackCaoG commented Apr 20, 2021

Uh oh!

JackCaoG commented Apr 20, 2021

Uh oh!

bdhirsh commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdhirsh commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackCaoG left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bdhirsh commented Apr 19, 2021 •

edited

Loading

bdhirsh commented Apr 21, 2021 •

edited

Loading

bdhirsh commented Apr 21, 2021 •

edited

Loading