[hop][inductor] track the dependency on unbacked symbols correctly with constant_args for hops #143456

ydwu4 · 2024-12-18T01:08:38Z

Stack from ghstack (oldest at bottom):

-> [hop][inductor] track the dependency on unbacked symbols correctly with constant_args for hops #143456

Before the PR, we're getting an undefined symbol error for output code when an unbacked symint is only used in the hop because we didn't correctly record the dependency of the unbacked symbols for hops and it gets DCEed accidentally.

This PR adds the symbol arguments to constant_args, where the dependencies can be correctly constructed when get_unbacked_symbol_uses is called to check constant_args.

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-12-18T01:08:42Z

🔗 Helpful Links

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

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

ROCM Infra failures during checkout of PyTorch

✅ You can merge normally! (1 Unrelated Failure)

As of commit 4df8697 with merge base f85e4c1 ():

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

inductor / cuda12.4-py3.10-gcc9-sm86 / test (inductor_timm, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (trunk failure)
levit_128

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

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]

torch/_inductor/ir.py

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]

…orrectly with constant_args for hops" Before the PR, we're getting an undefined symbol error for output code when an unbacked symint is **only** used in the hop because we didn't correctly record the dependency of the unbacked symbols for hops and it gets DCEed accidentally. This PR adds the symbol arguments to `constant_args`, where the dependencies can be correctly constructed when `get_unbacked_symbol_uses` is called to check constant_args. 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]

ghstack-source-id: e3f31d9 Pull Request resolved: #143456

desertfire · 2025-01-24T15:12:40Z

torch/_inductor/ir.py

 class Conditional(ExternKernel):
    predicate: Optional[IRNode] = None
-    operands: Optional[list[TensorBox]] = None
+    operands: Optional[List[Union[TensorBox, ShapeAsConstantBuffer]]] = None


List->list is a recent update, #145137. Please don't undo it.

torch/_inductor/ir.py

eellison

didn't look too closely bc failing tests, re-review when ready

…orrectly with constant_args for hops" Before the PR, we're getting an undefined symbol error for output code when an unbacked symint is **only** used in the hop because we didn't correctly record the dependency of the unbacked symbols for hops and it gets DCEed accidentally. This PR adds the symbol arguments to `constant_args`, where the dependencies can be correctly constructed when `get_unbacked_symbol_uses` is called to check constant_args. 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]

pytorchmergebot · 2025-01-30T22:59:08Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x e5e4391bb6a1c146b38b518f95e5f80bd66df733 returned non-zero exit code 1

Auto-merging test/inductor/test_control_flow.py
CONFLICT (content): Merge conflict in test/inductor/test_control_flow.py
error: could not apply e5e4391bb6a... [hop][inductor] support unbacked symint closure in inductor
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Details for Dev Infra team

Raised by workflow job

…orrectly with constant_args for hops" Before the PR, we're getting an undefined symbol error for output code when an unbacked symint is **only** used in the hop because we didn't correctly record the dependency of the unbacked symbols for hops and it gets DCEed accidentally. This PR adds the symbol arguments to `constant_args`, where the dependencies can be correctly constructed when `get_unbacked_symbol_uses` is called to check constant_args. 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]

ghstack-source-id: 58cbf9e Pull Request resolved: #143456

ydwu4 · 2025-01-31T18:22:02Z

@pytorchbot merge

pytorchmergebot · 2025-01-31T18:23:42Z

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

atalman · 2025-02-03T16:24:21Z

@pytorchmergebot revert -c nosignal -m "New tests are failing internally"

pytorchmergebot · 2025-02-03T16:25:47Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2025-02-03T16:26:01Z

@ydwu4 your PR has been successfully reverted.

…ectly with constant_args for hops (#143456)" This reverts commit 68a3635. Reverted #143456 on behalf of https://github.com/atalman due to New tests are failing internally ([comment](#143456 (comment)))

…orrectly with constant_args for hops" Before the PR, we're getting an undefined symbol error for output code when an unbacked symint is **only** used in the hop because we didn't correctly record the dependency of the unbacked symbols for hops and it gets DCEed accidentally. This PR adds the symbol arguments to `constant_args`, where the dependencies can be correctly constructed when `get_unbacked_symbol_uses` is called to check constant_args. 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]

ghstack-source-id: 2a0a5c4 Pull Request resolved: #143456

ydwu4 · 2025-02-04T18:37:00Z

@pytorchbot merge

pytorchmergebot · 2025-02-04T18:41:08Z

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

[hop] support unbacked symint closure in inductor

d841776

[ghstack-poisoned]

pytorch-bot bot added ciflow/inductor module: inductor labels Dec 18, 2024

ydwu4 added the topic: not user facing topic category label Dec 18, 2024

ydwu4 added 3 commits December 18, 2024 13:51

Update on "[hop] support unbacked symint closure in inductor"

4ceed2c

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]

Update on "[hop] support unbacked symint closure in inductor"

cec1d5c

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]

Update on "[hop] support unbacked symint closure in inductor"

9490d49

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]

ydwu4 mentioned this pull request Dec 19, 2024

[hop][BE] unify meta checking with check_meta_consistency #143545

Closed

Update on "[hop] support unbacked symint closure in inductor"

357673a

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]

ydwu4 mentioned this pull request Dec 19, 2024

[hop] fix unbacked_bindings meta for while_loop #143559

Closed

zou3519 reviewed Dec 19, 2024

View reviewed changes

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

ydwu4 added 5 commits December 19, 2024 14:42

Update on "[hop] support unbacked symint closure in inductor"

01d3601

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]

Update on "[hop] support unbacked symint closure in inductor"

0a2a126

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]

Update on "[hop] support unbacked symint closure in inductor"

f8448d6

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]

Update on "[hop] support unbacked symint closure in inductor"

8de08cb

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]

Update on "[hop] support unbacked symint closure in inductor"

f64d954

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]

ydwu4 changed the title ~~[hop] support unbacked symint closure in inductor~~ [hop][inductor] track the dependency on unbacked symbols correctly with constant_args for hops Jan 6, 2025

ydwu4 requested review from Chillee, eellison and zou3519 January 7, 2025 17:56

ydwu4 added a commit that referenced this pull request Jan 23, 2025

[hop][inductor] support unbacked symint closure in inductor

5c181c2

ghstack-source-id: e3f31d9 Pull Request resolved: #143456

desertfire reviewed Jan 24, 2025

View reviewed changes

eellison reviewed Jan 27, 2025

View reviewed changes

pytorchmergebot removed the merging label Jan 30, 2025

ydwu4 added a commit that referenced this pull request Jan 30, 2025

[hop][inductor] support unbacked symint closure in inductor

f0dff94

ghstack-source-id: 58cbf9e Pull Request resolved: #143456

pytorchmergebot added the merging label Jan 31, 2025

pytorchmergebot added the Merged label Jan 31, 2025

pytorchmergebot closed this in 68a3635 Jan 31, 2025

pytorchmergebot removed the merging label Jan 31, 2025

pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Feb 3, 2025

pytorchmergebot reopened this Feb 3, 2025

ydwu4 added a commit that referenced this pull request Feb 4, 2025

[hop][inductor] support unbacked symint closure in inductor

a476798

ghstack-source-id: 2a0a5c4 Pull Request resolved: #143456

pytorchmergebot added the merging label Feb 4, 2025

pytorchmergebot closed this in b0fe975 Feb 4, 2025

pytorchmergebot removed the merging label Feb 4, 2025

github-actions bot deleted the gh/ydwu4/193/head branch March 7, 2025 02:08

[hop][inductor] track the dependency on unbacked symbols correctly with constant_args for hops #143456

[hop][inductor] track the dependency on unbacked symbols correctly with constant_args for hops #143456

Uh oh!

Conversation

ydwu4 commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Dec 18, 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/143456

❗ 1 Active SEVs

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

Uh oh!

desertfire Jan 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

pytorchmergebot commented Jan 30, 2025

Merge failed

Uh oh!

ydwu4 commented Jan 31, 2025

Uh oh!

pytorchmergebot commented Jan 31, 2025

Merge started

Uh oh!

atalman commented Feb 3, 2025

Uh oh!

pytorchmergebot commented Feb 3, 2025

Uh oh!

pytorchmergebot commented Feb 3, 2025

Uh oh!

ydwu4 commented Feb 4, 2025

Uh oh!

pytorchmergebot commented Feb 4, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ydwu4 commented Dec 18, 2024 •

edited

Loading

pytorch-bot bot commented Dec 18, 2024 •

edited

Loading