cms-sw / cmssw-config

CMSSW build configuration
4 stars 14 forks source link

Fix rocm compilation of dev.cc files #107

Closed ariostas closed 7 months ago

ariostas commented 7 months ago

Fixed the issue described in #105. I just changed an if statement to make it match the cuda logic.

https://github.com/cms-sw/cmssw-config/blob/95cbc7a0514813eb7797404d5f3ce1615ec40277/SCRAM/GMake/Makefile.cuda#L87

Closes #105, closes https://github.com/cms-sw/cmssw/issues/44506.

cmsbuild commented 7 months ago

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

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

cmsbuild commented 7 months ago

cms-bot internal usage

ariostas commented 7 months ago

Oops sorry @smuzaffar, I didn't see you had submitted a PR to fix this. Please close this PR if you prefer the other fix.

smuzaffar commented 7 months ago

test parameters:

smuzaffar commented 7 months ago

please test

smuzaffar commented 7 months ago

@ariostas , this change was what I tried first but then looking closely I noticed that initially we wanted to use hipcc as linker for rocm libraries that is why I came up with #106. But looks like #106 is still not complete. Change in this PR should fix #105. Once tests here are passed then I will merge it.

I ned to check with @fwyzard why we want to use hipcc for rocm libraries?

fwyzard commented 7 months ago

I ned to check with @fwyzard why we want to use hipcc for rocm libraries?

IIRC, hipcc is needed to link the device code included in the object files.

cmsbuild commented 7 months ago

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3be6e0/38472/summary.html COMMIT: 754e4965367b86cbd721b52fc3e6db6d0cf848f2 CMSSW: CMSSW_14_1_X_2024-03-27-1100/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw-config/107/38472/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3be6e0/38472/git-recent-commits.json https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3be6e0/38472/git-merge-result

Comparison Summary

Summary:

smuzaffar commented 7 months ago

+external