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

add sve capability check at runtime #2876

Merged
merged 6 commits into from
Aug 28, 2024

Conversation

rakshithgb-fujitsu
Copy link
Contributor

Description

oneDAL is currently forced to use only SVE on arm platforms, with this new additional check it will validate the presence of SVE.

It checks for the SVE using hardware capabilities mentioned here - https://www.kernel.org/doc/html/v5.15/arm64/sve.html

This reverts commit 141d8e9.

cpp/oneapi/dal/graph/undirected_adjacency_vector_graphqqq#
@rakshithgb-fujitsu
Copy link
Contributor Author

rakshithgb-fujitsu commented Aug 23, 2024

@napetrov @keeranroth have a look at this PR, this lays the base to push SVE tuned kernels.

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.

Looks good. Just thinking, if SVE is not supposed to be Arm specific, is it worth renaming the macro from TARGET_ARM to TARGET_AARCH64 to show that it's the ISA that is being checked?

.ci/scripts/clang-format.sh Outdated Show resolved Hide resolved
@rakshithgb-fujitsu
Copy link
Contributor Author

Looks good. Just thinking, if SVE is not supposed to be Arm specific, is it worth renaming the macro from TARGET_ARM to TARGET_AARCH64 to show that it's the ISA that is being checked?

I think its better to keep arm, eventually when neon support is added it can be the default path.

@@ -20,9 +20,11 @@ echo "Starting format check..."

RETURN_CODE=0

CLANG_FORMAT_EXE=${CLANG_FORMAT_EXE:-clang-format-14}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to set specific version here as default? Because we generally do not update this file, the default clang-format option would be used which provides some flexibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I observed this issue since the current CI uses clang-format-14, our systems had newer versions. The newer formatting was not compatible with clang-format-14 and hence failing the CI check. This allows to use the default version based on CI version.

@Alexsandruss
Copy link
Contributor

/intelci: run

@Alexsandruss Alexsandruss merged commit 42fb399 into oneapi-src:main Aug 28, 2024
17 of 18 checks passed
@rakshithgb-fujitsu rakshithgb-fujitsu deleted the arm_sve_check branch August 29, 2024 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants