[inductor] bugfix: keep WeakDeps (WAR deps) during fusion by v0i0 · Pull Request #162316 · pytorch/pytorch

v0i0 · 2025-09-06T00:29:38Z

Stack from ghstack (oldest at bottom):

-> [inductor] bugfix: keep WeakDeps (WAR deps) during fusion #162316

fixes #159855, was not triggered in other tests since it took
more than one round of fusion to get to the problematic code
which prunes WeakDeps. The WeakDeps are important to inhibit
fusion of kernels that read/write data into mutated buffers
with different indexing.

We modify the code to a) always prune before fusion, rather
than after, which improves its coverage and makes our basic
vertical fusion tests surface this issue as well and b)
check whether the weak dep is fusable before eliminating it
(which basically means checking that the producing code and
the consuming code are sufficiently compatible).

The tests that trigger this with change (a) is:
test_fusing_write_into_disjoint_read introduced in #118210.

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

…EMOVE during fusion [ghstack-poisoned]

pytorch-bot · 2025-09-06T00:29:41Z

🔗 Helpful Links

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

📄 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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 9da88e3 with merge base a89d5e9 ():

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

trunk / win-vs2022-cpu-py3 / test (default, 1, 3, windows.4xlarge.nonephemeral) (gh) (similar failure)
'Test'

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

…EMOVE during fusion ghstack-source-id: 7c5791f Pull Request resolved: #162316

v0i0 · 2025-09-06T00:31:10Z

fixes #159855 will try to understand the surrounding code a bit more to see if there is a better way

eellison

Do you have a repro ? For write after read, we should be checking that the indexing expressions are exactly equal:

pytorch/torch/_inductor/scheduler.py

Lines 4142 to 4178 in 5fa1f52

    
           # StarDep doesn't match MemoryDep, different indices don't match 
        
           # However, broadcasting sometimes strips dimensions, and if that's the case 
        
           # we still can match unmet dep 
        
           # if there's indirect indexing, don't match it 
        
           def fusable_read_and_write(self, read: Dep, write: MemoryDep) -> bool: 
        
               if isinstance(read, MemoryDep): 
        
                   read_name = self.mutation_renames.get(read.name, read.name) 
        
                   if ( 
        
                       read_name != write.name 
        
                       or free_symbol_is_type(read.index, SymT.TMP) 
        
                       or free_symbol_is_type(write.index, SymT.TMP) 
        
                   ): 
        
                       return False 
        
                   if config.loop_ordering_after_fusion and read.num_vars != write.num_vars: 
        
                       # Need merge loops if we do loop ordering after fusion since 
        
                       # we have not merged the loops yet when creating the scheduler 
        
                       # nodes. 
        
                       read = read.normalize() 
        
                       write = write.normalize() 
        
                   return ( 
        
                       read.index == write.index 
        
                       and len(read.size) >= len(write.size) 
        
                       and read.size[: len(write.size)] == write.size 
        
                   ) 
        
               elif isinstance(read, StarDep): 
        
                   read_name = self.mutation_renames.get(read.name, read.name) 
        
                   write_name = self.mutation_renames.get(write.name, write.name) 
        
                   if ( 
        
                       read.mode == write.mode 
        
                       and write.mode is not None 
        
                       and read_name == write_name 
        
                   ): 
        
                       return True 
        
               return False

Original pr here: #118210

v0i0 · 2025-09-08T17:04:49Z

@eellison i have a repro, it is just a little complicated (not quite sure yet how to get a smaller repro, the obvious thing does not work). It seems to only trigger if node2 has MutationLayoutSHOULDREMOVE, which seems to hide the fact that the input buffer of node1 is the same as the output buffer of node2.

…youtSHOULDREMOVE during fusion" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

…EMOVE during fusion ghstack-source-id: ea3bff0 Pull Request resolved: #162316

…youtSHOULDREMOVE during fusion" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

…EMOVE during fusion ghstack-source-id: 01a8eb2 Pull Request resolved: #162316

v0i0 · 2025-09-08T20:30:26Z

@pytorchbot label "topic: not user facing"

…youtSHOULDREMOVE during fusion" cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

…EMOVE during fusion ghstack-source-id: 0bbab8e Pull Request resolved: #162316

eellison

nice ! for posterity - mind updating the commit message with details abt bug/fix ? (and what test case exercises this with your changes)

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: 0bbab8e Pull Request resolved: #162316

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: effb654 Pull Request resolved: #162316

v0i0 · 2025-09-09T18:21:03Z

@pytorchbot merge

pytorchmergebot · 2025-09-09T18:22:51Z

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 · 2025-09-09T19:27:01Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / linux-jammy-cuda12.8-py3.10-gcc11 / test (default, 5, 5, linux.g6.4xlarge.experimental.nvidia.gpu)

Details for Dev Infra team

Raised by workflow job

v0i0 · 2025-09-11T21:11:45Z

https://github.com/pytorch/pytorch/actions/runs/17657418702

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: 46130ea Pull Request resolved: #162316

v0i0 · 2025-09-12T14:30:46Z

fixed up the foreach heuristic a bit better, we now only treat them different in the sense that if the weekdep is fusable into more than one of the subnodes, we retain it. this feels like the right way to do it?

regression again: https://github.com/pytorch/pytorch/actions/runs/17667209063

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: e3c79c0 Pull Request resolved: #162316

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: 078ec9f Pull Request resolved: #162316

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben [ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: a9015a5 Pull Request resolved: #162316

shunting314 · 2025-09-18T17:02:32Z

I stated a perf test for h100 here. @v0i0 . Perf looks fine. Timm has perf regression probably because more models pass the accuracy test.

v0i0 · 2025-09-19T17:48:26Z

i checked out what the biggest regression was according to the regression run, and it neither reproduced nor did tlparse diff shown a difference in inductor. as such i'll fix it up and it should be good to go

[ghstack-poisoned]

fixes #159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in #118210. ghstack-source-id: 1754708 Pull Request resolved: #162316

v0i0 · 2025-09-21T13:00:33Z

@pytorchbot merge

pytorchmergebot · 2025-09-21T13:02: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

…2316) fixes pytorch#159855, was not triggered in other tests since it took more than one round of fusion to get to the problematic code which prunes WeakDeps. The WeakDeps are important to inhibit fusion of kernels that read/write data into mutated buffers with different indexing. We modify the code to a) always prune before fusion, rather than after, which improves its coverage and makes our basic vertical fusion tests surface this issue as well and b) check whether the weak dep is fusable before eliminating it (which basically means checking that the producing code and the consuming code are sufficiently compatible). The tests that trigger this with change (a) is: test_fusing_write_into_disjoint_read introduced in pytorch#118210. Pull Request resolved: pytorch#162316 Approved by: https://github.com/eellison, https://github.com/mlazos, https://github.com/shunting314

[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDR…

5fa1f52

…EMOVE during fusion [ghstack-poisoned]

pytorch-bot bot added ciflow/inductor module: inductor labels Sep 6, 2025

v0i0 added a commit that referenced this pull request Sep 6, 2025

[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDR…

2c84016

…EMOVE during fusion ghstack-source-id: 7c5791f Pull Request resolved: #162316

v0i0 requested a review from eellison September 8, 2025 16:39

eellison reviewed Sep 8, 2025

View reviewed changes

v0i0 added a commit that referenced this pull request Sep 8, 2025

[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDR…

2a648dd

…EMOVE during fusion ghstack-source-id: ea3bff0 Pull Request resolved: #162316

v0i0 added a commit that referenced this pull request Sep 8, 2025

[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDR…

96a05be

…EMOVE during fusion ghstack-source-id: 01a8eb2 Pull Request resolved: #162316

pytorch-bot bot added the topic: not user facing topic category label Sep 8, 2025

v0i0 added a commit that referenced this pull request Sep 8, 2025

[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDR…

c2333b8

…EMOVE during fusion ghstack-source-id: 0bbab8e Pull Request resolved: #162316

v0i0 requested a review from eellison September 8, 2025 21:56

eellison approved these changes Sep 8, 2025

View reviewed changes

v0i0 changed the title ~~[inductor] bugfix: consider WAR dependency from MutatingLayoutSHOULDREMOVE during fusion~~ [inductor] bugfix: keep WeakDeps (WAR deps) during fusion Sep 8, 2025

eellison mentioned this pull request Sep 9, 2025

[inductor] keep weakdep for mutation with different index #162459

Closed

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 9, 2025

pytorchmergebot added the merging label Sep 9, 2025

eellison mentioned this pull request Sep 10, 2025

[inductor] turn on loaf (for oss) by default #162030

Closed

v0i0 mentioned this pull request Sep 11, 2025

the index_put_ is not working as expected under torch.compile #162146

Closed

mlazos approved these changes Sep 18, 2025

View reviewed changes

Update

9da88e3

[ghstack-poisoned]

shunting314 self-requested a review September 19, 2025 18:16

shunting314 approved these changes Sep 19, 2025

View reviewed changes

pytorchmergebot added the merging label Sep 21, 2025

pytorchmergebot added the Merged label Sep 21, 2025

pytorchmergebot closed this in f34744d Sep 21, 2025

pytorchmergebot removed the merging label Sep 21, 2025

github-actions bot deleted the gh/v0i0/9/head branch October 22, 2025 02:15

	# StarDep doesn't match MemoryDep, different indices don't match
	# However, broadcasting sometimes strips dimensions, and if that's the case
	# we still can match unmet dep
	# if there's indirect indexing, don't match it
	def fusable_read_and_write(self, read: Dep, write: MemoryDep) -> bool:
	if isinstance(read, MemoryDep):
	read_name = self.mutation_renames.get(read.name, read.name)

	if (
	read_name != write.name
	or free_symbol_is_type(read.index, SymT.TMP)
	or free_symbol_is_type(write.index, SymT.TMP)
	):
	return False

	if config.loop_ordering_after_fusion and read.num_vars != write.num_vars:
	# Need merge loops if we do loop ordering after fusion since
	# we have not merged the loops yet when creating the scheduler
	# nodes.
	read = read.normalize()
	write = write.normalize()

	return (
	read.index == write.index
	and len(read.size) >= len(write.size)
	and read.size[: len(write.size)] == write.size
	)
	elif isinstance(read, StarDep):
	read_name = self.mutation_renames.get(read.name, read.name)
	write_name = self.mutation_renames.get(write.name, write.name)
	if (
	read.mode == write.mode
	and write.mode is not None
	and read_name == write_name
	):
	return True
	return False

Conversation

v0i0 commented Sep 6, 2025 • 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 Sep 6, 2025 • 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/162316

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

v0i0 commented Sep 6, 2025

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

v0i0 commented Sep 8, 2025

Uh oh!

v0i0 commented Sep 8, 2025

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

v0i0 commented Sep 9, 2025

Uh oh!

pytorchmergebot commented Sep 9, 2025

Merge started

Uh oh!

pytorchmergebot commented Sep 9, 2025

Merge failed

Uh oh!

v0i0 commented Sep 11, 2025

Uh oh!

v0i0 commented Sep 12, 2025

Uh oh!

shunting314 commented Sep 18, 2025

Uh oh!

v0i0 commented Sep 19, 2025

Uh oh!

v0i0 commented Sep 21, 2025

Uh oh!

pytorchmergebot commented Sep 21, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

v0i0 commented Sep 6, 2025 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Sep 6, 2025 •

edited

Loading