[PyTorch] Check `defined(aarch64) && !defined(CPU_CAPABILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)` #137722

swolchok · 2024-10-10T19:35:06Z

Stack from ghstack (oldest at bottom):

[PyTorch] Fix incorrect macOS 15.0 gating in MPS backend #138022
[PyTorch] Don't hardcode 4 * Vec::size() in vectorized_reduction #138014
-> [PyTorch] Check defined(__aarch64__) && !defined(CPU_CAPABILITY_SVE256) instead of defined(CPU_CAPABILITY_NEON) #137722

The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704

Differential Revision: D64197046

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

The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/) [ghstack-poisoned]

pytorch-bot · 2024-10-10T19:35:09Z

🔗 Helpful Links

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

📄 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 1f40b24 with merge base 0786b37 ():
💚 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 · 2024-10-10T19:35:20Z

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

swolchok · 2024-10-10T20:48:26Z

looks like this needs to be identical to the gating surrounding vec256_float_neon

swolchok · 2024-10-10T20:50:13Z

looks like this needs to be identical to the gating surrounding vec256_float_neon

specifically, 1) we just use __aarch64__ to gate that implementation, so we should follow suit here
2) we also need to check !defined(CPU_CAPABILITY_SVE256) because this apparently messes with SVE

…PABILITY_NEON" The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/) cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

facebook-github-bot · 2024-10-10T21:11:11Z

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

swolchok · 2024-10-11T01:25:10Z

hooray for green tests

abhishek-iitmadras · 2024-10-11T23:57:17Z

Hi @swolchok
Wouldn't it be better to define CPU_CAPABILITY_SVE instead of just CPU_CAPABILITY_SVE256?

swolchok · 2024-10-14T14:59:59Z

Hi @swolchok Wouldn't it be better to define CPU_CAPABILITY_SVE instead of just CPU_CAPABILITY_SVE256?

it is CPU_CAPABILITY_SVE256 because that is the gating surrounding vec256_float_neon.h: https://www.internalfb.com/code/fbsource/[0660504a24502363bb08b1b37504bc3ef6d878eb]/fbcode/caffe2/aten/src/ATen/cpu/vec/vec256/vec256.h?lines=10-15

…BILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)`" The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/) cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

facebook-github-bot · 2024-10-14T17:45:07Z

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

…BILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)`" The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/) cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

facebook-github-bot · 2024-10-15T18:26:53Z

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

malfet

This strips the ability of generating non-NEON accelerated code in torch.comple, which feels like a regression...
I.e. please undo changes to cpp_prefix.h/cpu_vec_isa.py

…h64__) && !defined(CPU_CAPABILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)`" The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/) cc jgong5 mingfeima XiaobingSuper sanchitintel ashokei jingxu10 voznesenskym penguinwu EikanWang Guobing-Chen zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang [ghstack-poisoned]

facebook-github-bot · 2024-10-15T19:51:26Z

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

…56)` instead of `defined(CPU_CAPABILITY_NEON)` Pull Request resolved: #137722 The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 ghstack-source-id: 248161061 @exported-using-ghexport Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/)

malfet

Looks good to me, thank you for the update

…56)` instead of `defined(CPU_CAPABILITY_NEON)` Pull Request resolved: pytorch/pytorch#137722 The CPU_CAPABILITY system is for rebuilding kernels multiple times with different vector ISA targets. CPU_CAPABILITY_NEON was not being used for that, just as an extra flag for inductor. As a result, CPU_CAPABILITY_NEON-gated code was unnecessarily unavailable outside inductor. Fixes #137704 ghstack-source-id: 247383134 @exported-using-ghexport Differential Revision: [D64197046](https://our.internmc.facebook.com/intern/diff/D64197046/)

…8014) This will break once we support 128-bit vectors, and there's no reason to do it. Differential Revision: [D64421982](https://our.internmc.facebook.com/intern/diff/D64421982/) Pull Request resolved: #138014 Approved by: https://github.com/malfet, https://github.com/Skylion007 ghstack dependencies: #137722

facebook-github-bot · 2024-10-16T20:51:06Z

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

The ifdef as written just checks if the macOS 15.0-capable SDK is being used. You also need a runtime gate to make sure macOS 15 is in use. Differential Revision: [D64429453](https://our.internmc.facebook.com/intern/diff/D64429453/) Pull Request resolved: #138022 Approved by: https://github.com/Skylion007, https://github.com/malfet ghstack dependencies: #137722, #138014

pytorch-bot bot added ciflow/inductor module: cpu CPU specific problem (e.g., perf, algorithm) module: inductor labels Oct 10, 2024

facebook-github-bot added the fb-exported label Oct 10, 2024

swolchok added the topic: not user facing topic category label Oct 10, 2024

swolchok requested review from jgong5 and malfet October 10, 2024 19:59

swolchok changed the title ~~[PyTorch] Check if __ARM_NEON is defined instead of CPU_CAPABILITY_NEON~~ [PyTorch] Check defined(__aarch64__) && !defined(CPU_CAPABILITY_SVE256) instead of defined(CPU_CAPABILITY_NEON) Oct 10, 2024

jgong5 approved these changes Oct 11, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 11, 2024

swolchok mentioned this pull request Oct 15, 2024

[PyTorch] Hook up fp16_gemv_trans to gemv fast path for non-aarch64 architectures #138005

Closed

malfet requested changes Oct 15, 2024

View reviewed changes

swolchok mentioned this pull request Oct 15, 2024

[PyTorch] Don't hardcode 4 * Vec::size() in vectorized_reduction #138014

Closed

malfet approved these changes Oct 15, 2024

View reviewed changes

swolchok mentioned this pull request Oct 15, 2024

[PyTorch] Fix incorrect macOS 15.0 gating in MPS backend #138022

Closed

pytorchmergebot added the Merged label Oct 16, 2024

pytorchmergebot closed this in a12c859 Oct 16, 2024

[PyTorch] Check defined(__aarch64__) && !defined(CPU_CAPABILITY_SVE256) instead of defined(CPU_CAPABILITY_NEON) #137722

[PyTorch] Check defined(__aarch64__) && !defined(CPU_CAPABILITY_SVE256) instead of defined(CPU_CAPABILITY_NEON) #137722

Uh oh!

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Oct 10, 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/137722

✅ No Failures

Uh oh!

facebook-github-bot commented Oct 10, 2024

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

facebook-github-bot commented Oct 10, 2024

Uh oh!

swolchok commented Oct 11, 2024

Uh oh!

abhishek-iitmadras commented Oct 11, 2024

Uh oh!

swolchok commented Oct 14, 2024

Uh oh!

facebook-github-bot commented Oct 14, 2024

Uh oh!

facebook-github-bot commented Oct 15, 2024

Uh oh!

malfet left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 15, 2024

Uh oh!

malfet left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Oct 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[PyTorch] Check `defined(aarch64) && !defined(CPU_CAPABILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)` #137722

[PyTorch] Check `defined(aarch64) && !defined(CPU_CAPABILITY_SVE256)` instead of `defined(CPU_CAPABILITY_NEON)` #137722

swolchok commented Oct 10, 2024 •

edited

Loading

pytorch-bot bot commented Oct 10, 2024 •

edited

Loading

swolchok commented Oct 10, 2024 •

edited

Loading

swolchok commented Oct 10, 2024 •

edited

Loading