Skip to content
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.

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

Fixes the cpuinfo_get_max_arm_sve_length API bug on NON-SVE supported hardware. #258

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

maajidkhann
Copy link
Contributor

The current API - cpuinfo_get_max_arm_sve_length() when integrated into PyTorch works well on SVE supported hardware but when tested on Graviton2 (NON SVE hardware), it fails with the below error.

cpuinfo_error_on_graviton2

This PR fixes this bug on Non SVE supported hardware:

Changes:
cpuinfo_log_error (wrong change) -> cpuinfo_log_warning (Right change)

*The current API - cpuinfo_get_max_arm_sve_length()
when integarted into PyTorch and tested on Graviton2
(NON SVE hardware), it fails with an error.

Signed-off-by: maajidkhann <[email protected]>
@maajidkhann
Copy link
Contributor Author

@malfet Please review and merge this small change.

@ng-05
Copy link

ng-05 commented Sep 11, 2024

cpuinfo_log_error() does not terminate the execution. Can you please tell how replacing cpuinfo_log_error() with cpuinfo_log_waring() fixes the graviton2 test failure?

@maajidkhann
Copy link
Contributor Author

cpuinfo_log_error() does not terminate the execution. Can you please tell how replacing cpuinfo_log_error() with cpuinfo_log_waring() fixes the graviton2 test failure?

I made this change in cpuinfo and called the API from PyTorch on Graviton 2. I don't see it terminating the execution now like earlier.

cpuinfo_log_warning() doesn't terminate the execution, it just a warning. You can also see same is being used multiple times in the same file.
https://github.com/pytorch/cpuinfo/pull/258/files#diff-9536970666a7b45882a2719305c3eb2456da2f385d06ac01648dfa40e76ab311L82

https://github.com/pytorch/cpuinfo/pull/258/files#diff-9536970666a7b45882a2719305c3eb2456da2f385d06ac01648dfa40e76ab311L86

https://github.com/pytorch/cpuinfo/pull/258/files#diff-9536970666a7b45882a2719305c3eb2456da2f385d06ac01648dfa40e76ab311R53

@ng-05
Copy link

ng-05 commented Sep 11, 2024

cc: @fbarchard @digantdesai
can we please try to merge this fix. The pytorch aarch64 CI will keep on failing without this change

@digantdesai
Copy link
Contributor

digantdesai commented Sep 11, 2024

I don't think cpuinfo_log_error should terminate, stamping to unblock. FYI, we will need to update this in PyTorch too - https://github.com/pytorch/pytorch/tree/main/third_party

@digantdesai digantdesai merged commit a5ff6df into pytorch:main Sep 11, 2024
12 checks passed
@maajidkhann
Copy link
Contributor Author

I don't think cpuinfo_log_error should terminate, stamping to unblock. FYI, we will need to update this in PyTorch too - https://github.com/pytorch/pytorch/tree/main/third_party

Yes the cpuinfo needs to be updated in PyTorch to reflect the changes in PyTorch PR CI:
https://github.com/pytorch/cpuinfo/tree/fa1c679da8d19e1d87f20175ae1ec10995cd3dd3

@digantdesai Will you be able to update it?

malfet added a commit to pytorch/pytorch that referenced this pull request Oct 22, 2024
Spiritual cherry-pick of #138351 that picks pytorch/cpuinfo#258 into the branch
malfet added a commit to pytorch/pytorch that referenced this pull request Oct 22, 2024
Spiritual cherry-pick of #138351 that picks pytorch/cpuinfo#258 into the branch

Fixes #138333


Test Plan: `python -c "import torch"` finishes without any output on the screen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants