Skip to content

[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, Try #2 #137377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, Try #2 #137377

wants to merge 7 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Oct 4, 2024 •
edited
Loading

@swolchok
[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, …
d1b39cf 
…Try #2

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Oct 4, 2024 •
edited
Loading

🔗 Helpful Links

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

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

✅ No Failures

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

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

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

swolchok added a commit that referenced this pull request Oct 4, 2024
@swolchok
[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, …
1094d37 
…Try #2

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

ghstack-source-id: 246411194
Pull Request resolved: #137377
@swolchok swolchok mentioned this pull request Oct 4, 2024
Closed
@swolchok
Copy link
Contributor Author

swolchok commented Oct 4, 2024

@pytorchbot label "ciflow/linux-aarch64"

@pytorch-bot pytorch-bot bot added the ciflow/linux-aarch64 linux aarch64 CI workflow label Oct 4, 2024
@swolchok swolchok requested review from albanD and malfet and removed request for albanD October 4, 2024 22:31
@swolchok
Update on "[PyTorch] Port ExecuTorch bfdot improvement back to ATen B…
6b43497 
…lasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

@swolchok
Update on "[PyTorch] Port ExecuTorch bfdot improvement back to ATen B…
a95fad7 
…lasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

swolchok added a commit that referenced this pull request Oct 7, 2024
@swolchok
[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, …
17843ae 
…Try #2

Pull Request resolved: #137377

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .
ghstack-source-id: 246616406

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)
@albanD
Copy link
Collaborator

albanD commented Oct 7, 2024

What's the difference with the previous attempt?

@swolchok swolchok mentioned this pull request Oct 7, 2024
Closed
swolchok
swolchok commented Oct 8, 2024
View reviewed changes
aten/src/ATen/native/BlasKernel.cpp Outdated
}

// NOTE: The first attempt at landing BFDOT support with
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@albanD here is the difference

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ho thanks for the pointer!
Should we undef DOT_WITH_FP32_ARITH_TAIL_AFTER_MAIN_LOOP_BODY ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a .cpp file, so the macro isn't going to leak anywhere and it's not particularly necessary, but sure I can do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@swolchok swolchok added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 8, 2024
@swolchok
Update on "[PyTorch] Port ExecuTorch bfdot improvement back to ATen B…
b2687b6 
…lasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

@swolchok
Copy link
Contributor Author

swolchok commented Oct 8, 2024

will fix https://hud.pytorch.org/pr/pytorch/pytorch/137377#31249893287

@swolchok
more thorough GCC fix on "[PyTorch] Port ExecuTorch bfdot improvement…
9d9919e 
… back to ATen BlasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

@swolchok
undef macro on "[PyTorch] Port ExecuTorch bfdot improvement back to A…
88385cb 
…Ten BlasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

swolchok added a commit that referenced this pull request Oct 8, 2024
@swolchok
[PyTorch] Port ExecuTorch bfdot improvement back to ATen BlasKernel, …
2eed1d4 
…Try #2

Pull Request resolved: #137377

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .
ghstack-source-id: 246956192

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)
@swolchok
Copy link
Contributor Author

swolchok commented Oct 9, 2024

CI is 100% green; please review

@swolchok swolchok mentioned this pull request Oct 9, 2024
Closed
@swolchok
Update on "[PyTorch] Port ExecuTorch bfdot improvement back to ATen B…
fcb5288 
…lasKernel, Try #2"

ExecuTorch's fork of BlasKernel.cpp grew bfdot support, complete with demonstration that it helps. Port it back to PyTorch. First attempt was #136331 .

Differential Revision: [D63923166](https://our.internmc.facebook.com/intern/diff/D63923166/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D63923166

@malfet
Copy link
Contributor

malfet commented Oct 10, 2024

If CI is green, than sure, LGTM

This was referenced Oct 10, 2024
Closed
Closed
@swolchok
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

@github-actions github-actions bot deleted the gh/swolchok/649/head branch November 10, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malfet malfet malfet approved these changes

@albanD albanD Awaiting requested review from albanD

Assignees
No one assigned
Labels
ciflow/linux-aarch64 linux aarch64 CI workflow ciflow/trunk Trigger trunk jobs on your pull request fb-exported Merged topic: performance topic category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@swolchok @facebook-github-bot @albanD @malfet @pytorchmergebot