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

Enable Cross Compilation of Arm SVE on x86 Cl (Binary only) #2691

Merged
merged 26 commits into from
Mar 25, 2024

Conversation

rakshithgb-fujitsu
Copy link
Contributor

Subsequent to the merge of pull request #2614, which introduced ARM as a supported CPU platform, we have implemented preliminary support for cross-compiling ARM backend on the x86 continuous integration (Cl) system.
This enhancement allows for validating ARM builds.
What's available:

  1. A new Cl task is created for arm compilation on x86 hosts, this pipeline builds both daal and oneapi components for ARM.
  2. Introduction of cross-compilation variable across Cl build scripts (tbb.sh, openblas.sh & build.sh)

Limitations:

  1. Examples build and tests for both daal and oneapi is currently disabled.

@rakshithgb-fujitsu rakshithgb-fujitsu changed the title Arm SVE CI Enable Cross Compilation of Arm SVE on x86 Cl Mar 14, 2024
@rakshithgb-fujitsu rakshithgb-fujitsu changed the title Enable Cross Compilation of Arm SVE on x86 Cl Enable Cross Compilation of Arm SVE on x86 Cl (Binary only) Mar 14, 2024
@napetrov
Copy link
Contributor

/intelci: run

Co-authored-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
@napetrov
Copy link
Contributor

/intelci: run

.ci/env/apt.sh Outdated Show resolved Hide resolved
.ci/env/openblas.sh Outdated Show resolved Hide resolved
.ci/env/tbb.sh Outdated Show resolved Hide resolved
.ci/env/tbb.sh Outdated
else
arch_dir=${arch}
arch=${target_arch:-$(uname -m)}
if [ "${arch}" == "x86_64" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

else
    arch=${target_arch:-$(uname -m)}
    arch_dir=${arch_dir:-$(set_arch_dir "${target_arch}")}

Copy link
Contributor Author

@rakshithgb-fujitsu rakshithgb-fujitsu Mar 20, 2024

Choose a reason for hiding this comment

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

isn't this redundant?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was meaning to replace the if statements with the call to the function, which I assume was created to avoid the duplication of this logic

fi

TBB_VERSION="v2021.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't feel like the TBB version should be hard-coded. This should be picked up from the pkgconfig file in the installation

.ci/env/tbb.sh Show resolved Hide resolved
.ci/env/arm-toolchain.cmake Outdated Show resolved Hide resolved
set(CMAKE_SYSTEM_PROCESSOR aarch64)

# Set compiler flags and options for ARMv8-A architecture with SVE
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -march=armv8-a+sve")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be set in the CMake toolchain? The march flags are dealt with in the build steps of the various projects, so think that this isn't necessary. If it is necessary to set the march flags, this should be done outside of the toolchain file, though

Copy link
Contributor Author

@rakshithgb-fujitsu rakshithgb-fujitsu Mar 20, 2024

Choose a reason for hiding this comment

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

currently only tbb needs the toolchain file and since we will be compiling only for sve this can be used?

@napetrov
Copy link
Contributor

/intelci: run

@napetrov
Copy link
Contributor

@keeranroth @rakshithgb-fujitsu any comments/changes here? I do not have anything major from my side and we can merge once vi would pass on latest changes

@napetrov
Copy link
Contributor

/intelci: run

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.

Just one comment, but looks good apart from that

.ci/env/tbb.sh Outdated
else
arch_dir=${arch}
arch=${target_arch:-$(uname -m)}
if [ "${arch}" == "x86_64" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I was meaning to replace the if statements with the call to the function, which I assume was created to avoid the duplication of this logic

@napetrov
Copy link
Contributor

/intelci: run

@keeranroth
Copy link
Contributor

All looking good from my point of view, thanks

@napetrov
Copy link
Contributor

Looks there open BLAS pipeline failure @rakshithgb-fujitsu can you take a look?

@napetrov
Copy link
Contributor

/intelci: run

@rakshithgb-fujitsu
Copy link
Contributor Author

rakshithgb-fujitsu commented Mar 23, 2024

@napetrov looks like something is breaking in internal pipeline again, could you please have a look and let me know?

@ethanglaser
Copy link
Contributor

@napetrov looks like something is breaking in internal pipeline again, could you please have a look and let me know?

Just a sporadic failure, nothing meaningful - CI looks good.

@napetrov napetrov merged commit 8aefe60 into oneapi-src:main Mar 25, 2024
14 of 15 checks passed
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.

None yet

5 participants