8000 More granular CPU features detection by Vika-F · Pull Request #3170 · uxlfoundation/oneDAL · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

More granular CPU features detection #3170

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
Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

More granular CPU features detection #3170

wants to merge 20 commits into from

Conversation

Vika-F
Copy link
Contributor
@Vika-F Vika-F commented Apr 14, 2025 •
edited
Loading

Description

API for run-time CPU features detection was added to DAAL and oneDAL.
Following features were included into initial list:

  • SpeedStep
  • Turbo Boost
  • bfloat16
  • AVX512 VNNI
  • Turbo Boost Max 3.0

Checklist to comply with before moving PR from draft:

PR completeness and readability

  • I have reviewed my changes thoroughly before submitting this pull request.
  • I have commented my code, particularly in hard-to-understand areas.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have added a respective label(s) to PR if I have a permission for that.
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.

Performance

not applicable

@Vika-F Vika-F marked this pull request as ready for review April 17, 2025 09:49
Alexandr-Solovev
Alexandr-Solovev reviewed Apr 17, 2025
View reviewed changes
Copy link
Contributor
@Alexandr-Solovev Alexandr-Solovev left a comment

Choose a reason for hiding this comment

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

General question: Is it possible to add these specs in this info: https://github.com/uxlfoundation/oneDAL/blob/main/cpp%2Fdaal%2Fsrc%2Fservices%2Flibrary_version_info.cpp#L53
?
It could extend future profiler abilities

@Vika-F
Copy link
Contributor Author
Vika-F commented Apr 17, 2025 •
edited
Loading

@Alexandr-Solovev
I can do it yes, just to make this info printable on demand via cpu_info.
But the main goal of this PR is to allow following checks in the code of DAAL algorithms:

if (__daal_serv_cpu_feature_detect() & daal::CpuFeature::tb3)
{
     /// Do something if Turbo Boost Max 3.0 Technology is present on the machine
}
else
{
    /// Do something else if Turbo Boost Max 3.0 Technology is _not_ present on the machine
}

The modifications in cpu_info are needed just to be able to somehow validate that the checks are working actually.

@Vika-F
Copy link
Contributor Author
Vika-F commented Apr 23, 2025

General question: Is it possible to add these specs in this info: https://github.com/uxlfoundation/oneDAL/blob/main/cpp%2Fdaal%2Fsrc%2Fservices%2Flibrary_version_info.cpp#L53 ? It could extend future profiler abilities

My previous comment was premature.
Unfortunately, I cannot add this info directly into the LibraryVersionInfo struct as it breaks the ABI.

But the information about CPU features can be obtained or printed using oneDAL API as it is shown in this test:
https://github.com/uxlfoundation/oneDAL/blob/main/cpp/oneapi/dal/test/global_context.cpp#L135

@Vika-F
Copy link
Contributor Author
Vika-F commented Apr 23, 2025

/intelci: run

Alexsandruss
Alexsandruss requested changes Apr 24, 2025
View reviewed changes
keeranroth
keeranroth reviewed Apr 24, 2025
View reviewed changes
Copy link
Contributor
@keeranroth keeranroth left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. Just a nit on naming, but the rest looks good to me.

@Vika-F
Copy link
Contributor Author
Vika-F commented Apr 24, 2025

/intelci: run

@napetrov
Copy link
Contributor

Sorry for off topic - but things are now get to correct config and @rakshithgb-fujitsu @keeranroth you now proper approvers.
image

Would work on creating PR for codeowners updates so you would be automatically assigned as relievers for some of the aspects

@Vika-F
Copy link
Contributor Author
Vika-F commented Apr 25, 2025

Re-run CI: http://intel-ci.intel.com/f021d3db-fe7e-f156-a9a2-a4bf010d0e2d

@Vika-F
Merge pull request #47 from uxlfoundation/main
fec1c78 
Pull changes from main branch
@Vika-F
Copy link
Contributor Author
Vika-F commented May 5, 2025

/intelci: run

@Vika-F Vika-F requested a review from maria-Petrova as a code owner May 8, 2025 11:32
@Vika-F
Copy link
Contributor Author
Vika-F commented May 8, 2025

/intelci: run

A3E2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@napetrov napetrov napetrov left review comments

@david-cortes-intel david-cortes-intel david-cortes-intel left review comments

@Alexsandruss Alexsandruss Alexsandruss approved these changes

@Alexandr-Solovev Alexandr-Solovev Alexandr-Solovev approved these changes

@rakshithgb-fujitsu rakshithgb-fujitsu rakshithgb-fujitsu approved these changes

@keeranroth keeranroth keeranroth approved these changes

@KateBlueSky KateBlueSky Awaiting requested review from KateBlueSky

@avolkov-intel avolkov-intel Awaiting requested review from avolkov-intel

@maria-Petrova maria-Petrova Awaiting requested review from maria-Petrova

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Vika-F @napetrov @Alexsandruss @Alexandr-Solovev @rakshithgb-fujitsu @keeranroth @david-cortes-intel
0