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

New alpaka backend rules #110

Merged
merged 7 commits into from
Sep 13, 2024
Merged

New alpaka backend rules #110

merged 7 commits into from
Sep 13, 2024

Conversation

smuzaffar
Copy link
Contributor

Added new build rules to enable/disable alpaka backends (cuda/rocm). Also allow to change the cuda/rocm compute capabilities (see cms-sw/cmssw#45859 (comment) for details)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar for branch scramv3.

@aandvalenzuela, @cmsbuild, @iarspider, @smuzaffar can you please review it and eventually sign? Thanks.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 12, 2024

cms-bot internal usage

Comment on lines 2092 to 2093
Enable/Disable alpaka gpus backends if not support on local host. It also calls\n\
Also set native compute capabilities for the enabled alpaka backends.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in the description ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@smuzaffar
Copy link
Contributor Author

test parameters:

  • addpkg= FWCore/Framework

@smuzaffar
Copy link
Contributor Author

please test

SCRAM/GMake/Makefile.rules Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

Pull request #110 was updated.

@cmsbuild
Copy link
Contributor

Pull request #110 was updated.

@fwyzard
Copy link
Contributor

fwyzard commented Sep 12, 2024

When there is a change to the tool file, could you make scram print a message that the user should run scram b checkdeps ?

@cmsbuild
Copy link
Contributor

Pull request #110 was updated.

Comment on lines 46 to 54
# remove existing capabilities flags
sed -r -i -e '/ROCM_FLAGS/s/--offload-arch=gfx[0-9a-f]+//g' ${TOOL}.tmp
# remove empty ROCM_FLAGS lines
sed -r -i -e '/ROCM_FLAGS=" *"/d' ${TOOL}.tmp

#add support for the capabilities found on this machine
for CAP in $CAPS; do
sed -i -e "s#</client>#</client>\n <flags ROCM_FLAGS=\"--offload-arch=${CAP}\"/>#" ${TOOL}.tmp
done
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fwyzard , may be something like this should be better for cuda tool too

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean splitting the capabilities to one entry per line ?
OK for me, but we have to change the way the CUDA flags are defined in the tool files.

Copy link
Contributor

Choose a reason for hiding this comment

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

But if you mean adding it as a separate entry only when done by this script, sure, OK for me.

Copy link
Contributor Author

@smuzaffar smuzaffar Sep 12, 2024

Choose a reason for hiding this comment

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

yes, separate entry when done by the script. Basically it will remove all existing -gencode arch=compute_*,code=[sm_*,compute_*] entries from the tool file , delete any empty CUDA_FLAGS line and then add new entries on new line after </client>. This does not require changing existing cuda tool file and should also work if we decided to change the cuda file to have multiple CUDA_FLAGS lines

@cmsbuild
Copy link
Contributor

Pull request #110 was updated.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9cf212/41488/summary.html
COMMIT: 252074b
CMSSW: CMSSW_14_2_X_2024-09-11-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw-config/110/41488/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@smuzaffar
Copy link
Contributor Author

+externals

@smuzaffar smuzaffar merged commit b2fa6c4 into scramv3 Sep 13, 2024
10 checks passed
@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next scramv3 IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @rappoccio, @mandrenguyen, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

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.

3 participants