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

Fix rocm compilation of dev.cc files #107

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

ariostas
Copy link
Contributor

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

ifneq ($$(strip $$($(1)_cudafiles)),)

Closes #105, closes cms-sw/cmssw#44506.

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

cmsbuild commented Mar 27, 2024

cms-bot internal usage

@ariostas
Copy link
Contributor Author

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
Copy link
Contributor

test parameters:

  • full_cmssw = true

@smuzaffar
Copy link
Contributor

please test

@smuzaffar
Copy link
Contributor

@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
Copy link
Contributor

fwyzard commented Mar 27, 2024

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
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3be6e0/38472/summary.html
COMMIT: 754e496
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:

  • You potentially removed 222 lines from the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 3108 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297437
  • DQMHistoTests: Total failures: 202
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3297215
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -95.97000000000003 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 10224.0,... ): -2.742 KiB Physics/Top
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@smuzaffar
Copy link
Contributor

+external

@smuzaffar smuzaffar merged commit 6b9906c into cms-sw:scramv3 Mar 28, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants