[hierarchical-compilation][inductor] Support invoke_subgraph HOP #138031

anijain2305 · 2024-10-15T22:43:09Z

Stack from ghstack (oldest at bottom):

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov

[ghstack-poisoned]

pytorch-bot · 2024-10-15T22:43:13Z

🔗 Helpful Links

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

📄 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 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

[PRE-EMPTIVE] Experimenting with new runners linux.aws.a100 on inductor-perf-compare.yml

✅ You can merge normally! (1 Unrelated Failure)

As of commit 6f4a50b with merge base ec4ce09 ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

inductor-periodic / cuda12.1-py3.10-gcc9-sm80 / test (inductor_torchbench_smoketest_perf, 1, 1, linux.gcp.a100) (gh) (detected as infra flaky with no log or failing log classifier)

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

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

torch/_inductor/codegen/wrapper.py

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

eellison

Looks good. Couple questions on subgraph reuse when invoking with different sets of tensors. I think we should fix the stride issue I mentioned.

Also, general question, where are the sanity checks on when a subgraph can be reused for different inputs ? Say, different shapes, etc. is that higher in the stack ?

eellison · 2024-10-16T23:14:07Z

torch/_inductor/ir.py



+@ir_dataclass
+class InvokeSubgraph(ExternKernel):


Do we have anything in place that will require similar aliasing relationships ? would you mind adding a test that shows we dont try to say inplace an input in the subgraph ?

Or similarly, invoke a subgraph once with the same buffer appearing twice, and then with the subgraph invoked and two different buffers, and check that we dont dedupe the tensor arg internally.

#138036 is for aliasing and input mutation.

Dynamo will ensure to not even create an invoke_subgraph HOP in fx graph if there is mutation.

eellison · 2024-10-16T23:22:24Z

torch/_inductor/ir.py

+        fx_operands = V.graph.current_node.args[-1]
+        fake_operands = [x.meta["val"] for x in fx_operands]  # type: ignore[union-attr]


inductor is not required to match strides for all intermediaries. We could try to force strides to be equal for these inputs, or we can update the fake_operands to match the strides of the operands input. Generally, it's better to let inductor have flexibility. I guess this also an existing issue with Cond and While loop cc @aakhundov .

If the strides are potentially changing, we may not always be able to reuse the subgraph lowering and would need to do some checking.

This is also a reason to implement assert_sizes_strides haha - would manifest as silent incorrectness rn if we had divergence

Yes, Horace also mentioned. I am planning to add asserts back for the subgraph

Added code to match the stride here. Please check.

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

ghstack-source-id: e40b88e Pull Request resolved: #138031

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

anijain2305 · 2024-10-22T16:59:52Z

torch/_inductor/ir.py

+        operands = [
+            cls.require_exact_strides(x, fake_strides[idx])
+            for idx, x in enumerate(operands)
+        ]


@eellison This is the change you requested. I am not sure if this is the right way.

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

eellison · 2024-10-23T17:43:04Z

torch/_inductor/ir.py

+        # TODO(anijain2305) - Support symint inputs in future.
+        fake_strides = [fake_operand.stride() for fake_operand in fake_operands]


we expect sym expr inside inductor, not symints.. it's a bit confusing, something we should fix.

Can you convert the symints first:

pytorch/torch/_inductor/graph.py

Lines 1490 to 1493 in c272526

strides = [

s.node.expr if isinstance(s, torch.SymInt) else s

for s in strides

]

Also -require_exact_strides doesn't work great with aliases/NonOwningLayout.... this should mostly work for now though.

…ph HOP" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

anijain2305 · 2024-10-23T18:13:55Z

@pytorchbot merge

pytorchmergebot · 2024-10-23T18:15:40Z

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

…8031) Pull Request resolved: #138031 Approved by: https://github.com/eellison ghstack dependencies: #137538, #138036, #137965

[hierarchical-compilation][inductor] Support invoke_subgraph HOP

3c7eb0c

[ghstack-poisoned]

anijain2305 mentioned this pull request Oct 15, 2024

[dynamo][refactor] Refactor Wrap HOP to reuse it for invoke_subgraph #137555

Closed

anijain2305 mentioned this pull request Oct 15, 2024

[hierarchical-compilation][hop] Introduce invoke_subgraph #137538

Closed

pytorch-bot bot added ciflow/inductor module: inductor labels Oct 15, 2024

anijain2305 mentioned this pull request Oct 15, 2024

[hierarchical-compilation][invoke_subgraph] Graph break on input mutation or aliasing #138036

Closed

anijain2305 requested review from Chillee, aakhundov, eellison and zou3519 October 15, 2024 23:34

anijain2305 added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 15, 2024

aakhundov reviewed Oct 16, 2024

View reviewed changes

torch/_inductor/codegen/wrapper.py Show resolved Hide resolved

anijain2305 mentioned this pull request Oct 16, 2024

[does not work] General hop fake tensor fn registration #138096

Closed

eellison reviewed Oct 16, 2024

View reviewed changes

anijain2305 mentioned this pull request Oct 17, 2024

[dynamo][testing] Update AOTEagerandRecordGraphs backend #138231

Closed

anijain2305 added 2 commits October 17, 2024 10:33

anijain2305 mentioned this pull request Oct 17, 2024

[hops][refactor] Refactor the aliasing/mutation detection functions #138234

Closed

anijain2305 added 5 commits October 17, 2024 10:47

anijain2305 added 3 commits October 21, 2024 13:44

anijain2305 added a commit that referenced this pull request Oct 21, 2024

[hierarchical-compilation][inductor] Support invoke_subgraph HOP

4ffec0a

ghstack-source-id: e40b88e Pull Request resolved: #138031

anijain2305 commented Oct 22, 2024

View reviewed changes

anijain2305 added the topic: not user facing topic category label Oct 22, 2024

eellison approved these changes Oct 23, 2024

View reviewed changes

pytorchmergebot added the merging label Oct 23, 2024

pytorchmergebot added the Merged label Oct 23, 2024

pytorchmergebot closed this in dd4dd85 Oct 23, 2024

pytorchmergebot removed the merging label Oct 23, 2024

SamGinzburg pushed a commit that referenced this pull request Oct 28, 2024

[hierarchical-compilation][inductor] Support invoke_subgraph HOP (#13…

f9c9b2f

…8031) Pull Request resolved: #138031 Approved by: https://github.com/eellison ghstack dependencies: #137538, #138036, #137965

github-actions bot deleted the gh/anijain2305/546/head branch November 23, 2024 02:07

		fx_operands = V.graph.current_node.args[-1]
		fake_operands = [x.meta["val"] for x in fx_operands] # type: ignore[union-attr]

		# TODO(anijain2305) - Support symint inputs in future.
		fake_strides = [fake_operand.stride() for fake_operand in fake_operands]

	strides = [
	s.node.expr if isinstance(s, torch.SymInt) else s
	for s in strides
	]



		@ir_dataclass
		class InvokeSubgraph(ExternKernel):

[hierarchical-compilation][inductor] Support invoke_subgraph HOP #138031

[hierarchical-compilation][inductor] Support invoke_subgraph HOP #138031

Uh oh!

Conversation

anijain2305 commented Oct 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 15, 2024 • 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/138031

❗ 1 Active SEVs

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

eellison Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

eellison Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

eellison Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 Oct 17, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anijain2305 Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

eellison Oct 23, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 commented Oct 23, 2024

Uh oh!

pytorchmergebot commented Oct 23, 2024

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anijain2305 commented Oct 15, 2024 •

edited

Loading

pytorch-bot bot commented Oct 15, 2024 •

edited

Loading

anijain2305 Oct 22, 2024 •

edited

Loading