Add support for item() and nonzero() codegen in Inductor #109893

ezyang · 2023-09-22T16:18:58Z

Stack from ghstack (oldest at bottom):

This is another version of
#109262 that I think is more
harmonious with inductor design.

Signed-off-by: Edward Z. Yang ezyang@meta.com

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov @ColinPeppler @ngimel

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

pytorch-bot · 2023-09-22T16:19:00Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/109893

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit ef0c742 with merge base 75462fd ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: df40f51 Pull Request resolved: #109893

ezyang · 2023-09-22T20:38:56Z

Discussion from @Chillee is that he is roughly ok with the approach, but he wants us to setup dependencies between DynamicScalar and the downstream usages of it (i.e., whenever the unbacked symint shows up.) This should be done in the scheduler.

jansel · 2023-09-23T18:04:09Z

torch/_inductor/codegen/cpp.py

-            hint = V.graph.sizevars.size_hint(expr)
+            sym_hint = V.graph.sizevars.symbolic_hint(expr)
+            if not isinstance(sym_hint, (int, sympy.Integer)):
+                # Make a guess
+                hint = 8192
+            else:
+                hint = int(sym_hint)


With the changes to size_hint above, is this change still needed?

yeah, because we need an actual integer to run the rest of this code, IIUC

Couldn't we just call size_hint() because it looks like the same code?

I misunderstood your comment originally.

The size_hint above is defined in CppKernel and hard coded to sympy_product(self.call-ranges). So no, I cannot "just call size_hint()" to hit the same code, at minimum there needs to be a refactor.

But we should not do this refactor. There is no intrinsic reason why the hints here should actually be the same. They happen to be the same because I am just randomly picking numbers. In fact, they probably should not be the same: the hint in size_hint is the product of all call ranges, which should be guessed to be larger than the hint for a single range. I am hoping we can revisit the hint selection later when we can do things like (1) have upper bounds on the size of the unbacked symint, which can be used to drive the hint or (2) maybe come up with some better heuristics armed with actual slowness that we're looking to speed up.

torch/_inductor/ir.py

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ezyang · 2023-09-26T19:00:01Z

reupping for review since the dep addition changes are fairly major

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

pytorchmergebot · 2023-09-28T23:37:09Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

**Summary** Follow up #109893 which has issue in support of CPU as reported in #109897. This fix mainly includes 2 changes: - Current implementation of `rename_indexing` https://github.com/pytorch/pytorch/blob/10c646295d3512237cfb3ab44aa21dfcc9832441/torch/_inductor/codegen/common.py#L1023 only add symbol name start with `s` or `ps` into `kernel.args.sizevars`. However, `Unbacked symint` will start as `i`, so we extend the implementation of `rename_indexing` to support symbol start with `i`. - Currently, the internal loop index also name start as `i`. Since `i` has has been used as `Unbacked symint`, change the name to start with `x` which should align with trition. **Test Plan** ``` python -u -m pytest -s -v test_torchinductor_dynamic_shapes.py -k test_bool_mask_nobreak python -u -m pytest -s -v test_torchinductor_dynamic_shapes.py -k test_nonzero_size_factory_nobreak python -u -m pytest -s -v test_torchinductor_dynamic_shapes.py -k test_item_zeros_nobreak ``` Pull Request resolved: #110262 Approved by: https://github.com/ezyang, https://github.com/jgong5

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: c3044c0 Pull Request resolved: #110520

…SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

…SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

…ed SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

…ed SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

…ed SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

…ed SymInts" Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler [ghstack-poisoned]

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. #109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #110520 Approved by: https://github.com/jansel, https://github.com/ezyang

Summary: Unbacked SymInts can't get a `sizevars.size_hint` due to being data-dependent. pytorch#109893 has added a new `fallback` parameter to `sizevars.size_hint` to specify the fallback value in cases like unbacked SymInt. In this PR we add more of those. Test Plan: CI Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: pytorch#110520 Approved by: https://github.com/jansel, https://github.com/ezyang

Add support for item() and nonzero() codegen in Inductor

251c041

This is another version of #109262 that I think is more harmonious with inductor design. Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

github-actions bot requested review from SherlockNoMad, albanD, antoniojkim, bdhirsh, miladm, voznesenskym and wconstab September 22, 2023 16:19

github-actions bot added module: inductor module: dynamo ciflow/inductor labels Sep 22, 2023

ezyang requested review from Chillee, aakhundov, jansel and yf225 September 22, 2023 16:19

ezyang mentioned this pull request Sep 22, 2023

Unbacked symint support in cpu inductor backend #109897

Closed

jansel approved these changes Sep 23, 2023

View reviewed changes

ezyang mentioned this pull request Sep 24, 2023

Add .item() and .tolist() support in Dynamo/Inductor without graph break #109262

Closed

ezyang requested a review from jansel September 26, 2023 18:59

pytorchmergebot added Merged and removed merging labels Sep 28, 2023

pytorchmergebot closed this in d1a1312 Sep 28, 2023

leslie-fang-intel mentioned this pull request Sep 29, 2023

[Inductor] Enable the item() and nonzero() codegen test on CPU #110262

Closed

ezyang mentioned this pull request Sep 29, 2023

Inductor does not check input SymInt invariant on GraphModules passed in #104102

Closed

facebook-github-bot deleted the gh/ezyang/2370/head branch October 2, 2023 14:26

aakhundov mentioned this pull request Oct 4, 2023

Fix size_hint call sites failing on unbacked SymInts #110520

Closed

ezyang mentioned this pull request Oct 5, 2023

OpenNMT doesn't work with dynamic torch.compile #93468

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for item() and nonzero() codegen in Inductor #109893

Add support for item() and nonzero() codegen in Inductor #109893

Uh oh!

ezyang commented Sep 22, 2023 •

edited

Loading

Uh oh!

pytorch-bot bot commented Sep 22, 2023 •

edited

Loading

Uh oh!

ezyang commented Sep 22, 2023

Uh oh!

jansel Sep 23, 2023

Uh oh!

ezyang Sep 26, 2023

Uh oh!

jansel Sep 27, 2023

Uh oh!

ezyang Sep 27, 2023

Uh oh!

Uh oh!

ezyang commented Sep 26, 2023

Uh oh!

pytorchmergebot commented Sep 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Add support for item() and nonzero() codegen in Inductor #109893

Add support for item() and nonzero() codegen in Inductor #109893

Uh oh!

Conversation

ezyang commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/109893

✅ No Failures

Uh oh!

ezyang commented Sep 22, 2023

Uh oh!

jansel Sep 23, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

jansel Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Sep 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ezyang commented Sep 26, 2023

Uh oh!

pytorchmergebot commented Sep 28, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ezyang commented Sep 22, 2023 •

edited

Loading

pytorch-bot bot commented Sep 22, 2023 •

edited

Loading