Horizontally fuse input concatenation #108115

mlazos · 2023-08-29T01:05:37Z

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

pytorch-bot · 2023-08-29T01:05:40Z

🔗 Helpful Links

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

📄 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.

❗ 1 Merge Blocking SEVs

There is 1 active merge blocking SEVs. Please view them below:

(merge blocking) GH API issues are preventing multiple ci jobs to start.

If you must merge, use @pytorchbot merge -f.

✅ No Failures

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

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

torch/_inductor/ir.py

mlazos · 2023-08-29T21:36:51Z

@pytorchbot merge

pytorchmergebot · 2023-08-29T21:39: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

pytorchmergebot · 2023-08-29T21:39:13Z

Merge failed

Reason: 10 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

mlazos · 2023-08-29T22:47:43Z

@pytorchbot merge

pytorchmergebot · 2023-08-29T22:49:38Z

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

pytorchmergebot · 2023-08-29T23:25:55Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 2, 5, linux.g5.4xlarge.nvidia.gpu)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

mlazos · 2023-08-30T19:43:12Z

@pytorchbot merge

pytorchmergebot · 2023-09-02T04:50:10Z

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

pytorchmergebot · 2023-09-02T05:30:47Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 3, 5, linux.g5.4xlarge.nvidia.gpu)

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

mlazos · 2023-09-02T06:12:07Z

@pytorchbot merge

pytorchmergebot · 2023-09-02T06:15:34Z

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

pytorchmergebot · 2023-09-02T10:49:36Z

Merge failed

Reason: 1 jobs have failed, first few of them are: inductor / cuda12.1-py3.10-gcc9-sm86 / test (inductor_torchbench_dynamic, 1, 1, linux.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team

Raised by workflow job

aakhundov · 2023-09-02T16:39:19Z

@mlazos seems that this PR currently contains only the new is_dynamic function in _inductor/utils.py. I'm wondering if the changes utilizing the new function got lost while merging with main (I see _inductor/ir.py was mentioned in the review above)? Thanks!

torch/_inductor/lowering.py

aakhundov · 2023-09-04T07:48:15Z

torch/_inductor/ir.py

+                input_unwrapped = inputs[i].data
+
+            if (
+                input_unwrapped.is_input_buffer()


Do I get it right that the horizontal fusion of separate concat kernels into foreach is enabled only when the concat arguments are graph inputs? I'm wondering if this could be extended to the cases when the arguments are intermediate (realized) buffers in the graph. Thanks!

Yeah this is true, I'm curious about your use case because I think in most cases of intermediates the concat kernel rewrites the storage of the upstream ops to point to the concat buffer rather than launching a copy kernel. Let me know if there are cases where we copy anyway and I can remove this constraint.

Thanks for the explanation. In my use case, concat takes (many) arguments which are outputs of the torch.ops.aten.slice.Tensor ops. So my understanding is that the latter are already views (reading from a particular offset of the input to the slice ops). At the same time, the slices / views are not graph inputs. So the ConcatKernel actually ends up generating a bunch of copy kernels: one per each slice input.

I see, yeah I'll experiment with removing this constraint in a separate PR since this has had a lot of edge cases and I want the MRS folks to test this. It should just work ootb I think

mlazos · 2023-09-05T16:53:14Z

@pytorchbot merge

pytorchmergebot · 2023-09-05T16:55:26Z

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: Original commit changeset: f15956d96311 Original Phabricator Diff: D48996091 Test Plan: Reverting to Unbreak test Differential Revision: D49065517

Summary: Original commit changeset: f15956d96311 Original Phabricator Diff: D48996091 Test Plan: Reverting to Unbreak test Differential Revision: D49065517 Pull Request resolved: #108793 Approved by: https://github.com/Chillee

facebook-github-bot · 2023-09-08T05:15:07Z

This pull request has been reverted by 6c72604. To re-land this change, please open another pull request, assignthe same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).

Reland #108115 The main fix is to disallow nop nodes to be included in foreach scheduler nodes Pull Request resolved: #111437 Approved by: https://github.com/yanboliang

mlazos added 2 commits August 29, 2023 00:59

Horizontally fuse input concat

113c141

Added test

0563c1a

mlazos requested review from jansel and yanboliang August 29, 2023 01:05

pytorch-bot bot added the release notes: foreach_frontend release notes category label Aug 29, 2023

github-actions bot added module: inductor ciflow/inductor labels Aug 29, 2023

Merge with main

c0ca4fc

mlazos added the ciflow/trunk Trigger trunk jobs on your pull request label Aug 29, 2023

jansel requested changes Aug 29, 2023

View reviewed changes

torch/_inductor/ir.py Outdated Show resolved Hide resolved

Restrict to cuda and when there are multiple kernels

adbf8c1

mlazos requested a review from jansel August 29, 2023 21:13

jansel approved these changes Aug 29, 2023

View reviewed changes

pytorchmergebot added the merging label Aug 29, 2023

pytorchmergebot removed the merging label Aug 29, 2023

Unwrap views

68a745c

pytorchmergebot added the merging label Aug 29, 2023

pytorchmergebot removed the merging label Aug 29, 2023

mlazos added 3 commits August 30, 2023 03:29

Don't fuse dynamic inputs

9ade8fd

Merge branch 'main' into mlazos/fuse-cat

94e833b

Make is_dynamic more robust

7a1e2e8

pytorchmergebot added the merging label Aug 30, 2023

pytorchmergebot added the merging label Sep 2, 2023

pytorchmergebot removed the merging label Sep 2, 2023

mlazos added 3 commits September 2, 2023 05:53

Merge with main

a6ce38f

Some nodes don't have stride

d1628fd

Merge with main

0758b52

pytorchmergebot added the merging label Sep 2, 2023

pytorchmergebot removed the merging label Sep 2, 2023

mlazos added 2 commits September 3, 2023 22:37

Revert revert PR

2cfc28a

Merge branch 'main' into mlazos/fuse-cat

c7ecd25

aakhundov reviewed Sep 4, 2023

View reviewed changes

torch/_inductor/lowering.py Show resolved Hide resolved

aakhundov reviewed Sep 4, 2023

View reviewed changes

Finally have a robust is_dynamic impl?

dbdf70e

pytorchmergebot added the merging label Sep 5, 2023

pytorchmergebot removed the merging label Sep 5, 2023

pytorchmergebot closed this in 96d7407 Sep 5, 2023

mlazos mentioned this pull request Oct 17, 2023

[Reland] horizontal concat fusion #111437

Closed

github-actions bot deleted the mlazos/fuse-cat branch March 6, 2025 02:07

Horizontally fuse input concatenation #108115

Horizontally fuse input concatenation #108115

Uh oh!

Conversation

mlazos commented Aug 29, 2023 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 29, 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/108115

❗ 1 Merge Blocking SEVs

✅ No Failures

Uh oh!

Uh oh!

mlazos commented Aug 29, 2023

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge started

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge failed

Uh oh!

mlazos commented Aug 29, 2023

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge started

Uh oh!

pytorchmergebot commented Aug 29, 2023

Merge failed

Uh oh!

mlazos commented Aug 30, 2023

Uh oh!

pytorchmergebot commented Sep 2, 2023

Merge started

Uh oh!

pytorchmergebot commented Sep 2, 2023

Merge failed

Uh oh!

mlazos commented Sep 2, 2023

Uh oh!

pytorchmergebot commented Sep 2, 2023

Merge started

Uh oh!

pytorchmergebot commented Sep 2, 2023

Merge failed

Uh oh!

aakhundov commented Sep 2, 2023

Uh oh!

Uh oh!

aakhundov Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

mlazos Sep 4, 2023

Choose a reason for hiding this comment

Uh oh!

aakhundov Sep 5, 2023

Choose a reason for hiding this comment

Uh oh!

mlazos Sep 5, 2023

Choose a reason for hiding this comment

Uh oh!

mlazos commented Sep 5, 2023

Uh oh!

pytorchmergebot commented Sep 5, 2023

Merge started

Uh oh!

facebook-github-bot commented Sep 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mlazos commented Aug 29, 2023 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Aug 29, 2023 •

edited

Loading