[inductor] Make sure unfuse_addmm and addmm patterns don't overlap #110235

peterbell10 · 2023-09-28T18:10:57Z

Stack from ghstack (oldest at bottom):

Inductor has two opposing patterns,

addmm -> add + mm
add + mm -> addmm

This uses the extra_check to disable the addmm fusion pattern when the
heuristic to unfuse add is met, for consistency.

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

pytorch-bot · 2023-09-28T18:10:59Z

🔗 Helpful Links

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

📄 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 fab9c1e with merge base 419ec3b ():
💚 Looks good so far! There are no failures yet. 💚

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

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. [ghstack-poisoned]

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. ghstack-source-id: 51dde8a7d748c44ce3e04d0f733dba3c14dc539a Pull Request resolved: #110235

…verlap" Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. ghstack-source-id: 19fb7d152103d3967d79f47ec9ca020705c3e19b Pull Request resolved: #110235

… overlap" Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. ghstack-source-id: 3f78107ecdb5d1a4df52b369d2e11241b845f8f8 Pull Request resolved: #110235

peterbell10 · 2023-09-29T12:44:51Z

test/inductor/test_pattern_matcher.py

            e1, e2 = fn(*args)
            a1, a2 = torch.compile(fn)(*args)
            torch.testing.assert_close(a1, e1)
            torch.testing.assert_close(a2, e2)
-            self.assertEqual(counters["inductor"]["pattern_matcher_count"], 2)
-            self.assertEqual(counters["inductor"]["pattern_matcher_nodes"], 4)
+            count, nodes = (2, 4) if should_fuse else (0, 0)


Note that these cases weren't actually fused previously, it's just that the pattern replaced them with a lowering that did add + mm.

lezcano

Fair enough. I think the code has a preexisting issue that we should fix tho.

torch/_inductor/fx_passes/post_grad.py

… overlap" Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. ghstack-source-id: 2bb268e61749862f7c32f6c7efb40673cf0189f3 Pull Request resolved: #110235

lezcano

Even better

peterbell10 · 2023-09-29T13:51:28Z

@pytorchbot merge

pytorchmergebot · 2023-09-29T13:53:39Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

peterbell10 · 2023-09-29T13:58:58Z

@pytorchbot merge

pytorchmergebot · 2023-09-29T14:00:54Z

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

eellison

Looks good just one comment about checking for input being a tensor

eellison · 2023-09-29T14:35:54Z

torch/_inductor/fx_passes/post_grad.py

-def addmm(match, mat1, mat2, inp):
-    if isinstance(inp, ir.TensorBox):
-        inp_shape = inp.get_size()
-        matched = len(inp_shape) <= 2
-        mm_shape = shape_of_mm(mat1, mat2)
-        for i, m in zip(inp_shape, mm_shape):
-            matched &= i == 1 or i == m
-    else:  # inp is a Number
-        matched = False


Nice to move this away from graph lowering pattern, this was overdue..

eellison · 2023-09-29T14:38:17Z

torch/_inductor/fx_passes/post_grad.py

+    return not should_prefer_unfused_addmm(match)
+
+
+@register_graph_pattern(


cc @yanboliang @jansel we should have some sort of commutative concept that would avoid this duplication

eellison · 2023-09-29T14:40:31Z

torch/_inductor/fx_passes/post_grad.py

+    if not isinstance(inp, torch.fx.Node):
+        return False  # Input is a number


i've made this check before which was fixed by #108160, you can have a fx.Node input which is a SymInt/SymFloat

…atterns don't overlap" Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler [ghstack-poisoned]

Inductor has two opposing patterns, ``` addmm -> add + mm add + mm -> addmm ``` This uses the `extra_check` to disable the addmm fusion pattern when the heuristic to unfuse add is met, for consistency. ghstack-source-id: f1ea08f1c736ada4dc88545dd0965753bbaf7bf2 Pull Request resolved: #110235

pytorchmergebot · 2023-09-29T15:34:03Z

Merge failed

Reason: New commits were pushed while merging. Please rerun the merge command.

Details for Dev Infra team

Raised by workflow job

peterbell10 · 2023-09-29T15:34:17Z

@pytorchbot merge

pytorchmergebot · 2023-09-29T15:37:05Z

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

peterbell10 mentioned this pull request Sep 28, 2023

[dynamo] Convert dtype arguments as well as inputs in cast_to_fp64 #110232

Closed

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

pytorchbot added the open source label Sep 28, 2023

peterbell10 marked this pull request as ready for review September 29, 2023 12:39

peterbell10 requested review from lezcano and eellison September 29, 2023 12:40

peterbell10 commented Sep 29, 2023

View reviewed changes

lezcano reviewed Sep 29, 2023

View reviewed changes

torch/_inductor/fx_passes/post_grad.py Outdated Show resolved Hide resolved

lezcano approved these changes Sep 29, 2023

View reviewed changes

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

pytorchmergebot added the merging label Sep 29, 2023

pytorchmergebot removed the merging label Sep 29, 2023

peterbell10 added the topic: not user facing topic category label Sep 29, 2023

pytorchmergebot added the merging label Sep 29, 2023

eellison approved these changes Sep 29, 2023

View reviewed changes

pytorchmergebot removed the merging label Sep 29, 2023

pytorchmergebot added the merging label Sep 29, 2023

pytorchmergebot added Merged and removed merging labels Sep 29, 2023

pytorchmergebot closed this in bc047ec Sep 29, 2023

facebook-github-bot deleted the gh/peterbell10/625/head branch October 3, 2023 14:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[inductor] Make sure unfuse_addmm and addmm patterns don't overlap #110235

[inductor] Make sure unfuse_addmm and addmm patterns don't overlap #110235

peterbell10 commented Sep 28, 2023 •

edited by pytorch-bot bot

pytorch-bot bot commented Sep 28, 2023 •

edited

peterbell10 Sep 29, 2023

lezcano left a comment

lezcano left a comment

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

eellison left a comment

eellison Sep 29, 2023

eellison Sep 29, 2023

eellison Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

		return not should_prefer_unfused_addmm(match)


		@register_graph_pattern(

		if not isinstance(inp, torch.fx.Node):
		return False # Input is a number

[inductor] Make sure unfuse_addmm and addmm patterns don't overlap #110235

[inductor] Make sure unfuse_addmm and addmm patterns don't overlap #110235

Conversation

peterbell10 commented Sep 28, 2023 • edited by pytorch-bot bot

pytorch-bot bot commented Sep 28, 2023 • edited

🔗 Helpful Links

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

✅ No Failures

peterbell10 Sep 29, 2023

Choose a reason for hiding this comment

lezcano left a comment

Choose a reason for hiding this comment

lezcano left a comment

Choose a reason for hiding this comment

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

Merge failed

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

Merge started

eellison left a comment

Choose a reason for hiding this comment

eellison Sep 29, 2023

Choose a reason for hiding this comment

eellison Sep 29, 2023

Choose a reason for hiding this comment

eellison Sep 29, 2023

Choose a reason for hiding this comment

pytorchmergebot commented Sep 29, 2023

Merge failed

peterbell10 commented Sep 29, 2023

pytorchmergebot commented Sep 29, 2023

Merge started

peterbell10 commented Sep 28, 2023 •

edited by pytorch-bot bot

pytorch-bot bot commented Sep 28, 2023 •

edited