Skip to content

Conversation

@smuzaffar
Copy link
Contributor

@smuzaffar smuzaffar commented Feb 26, 2025

In GCC14 IBs, we have many warning like [a]. This PR proposes to use std::atomic_ref. As simple test like [b] show that it works.

FYI @fwyzard @VinInn

[a]

In file included from src/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h:4,
                 from src/HeterogeneousCore/CUDAUtilities/test/HistoContainer_t.cpp:7:
src/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h: In instantiation of 'static uint32_t cms::cuda::OneToManyAssoc<I, ONES, SIZE>::atomicIncrement(Counter&) [with I = unsigned int; int ONES = 129; int SIZE = -1; uint32_t = unsigned int; Counter = unsigned int]':
src/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h:141:30:   required from 'void cms::cuda::HistoContainer<T, NBINS, SIZE, S, I, NHISTS>::count(T) [with T = short int; unsigned int NBINS = 128; int SIZE = -1; unsigned int S = 16; I = unsigned int; unsigned int NHISTS = 1]'
  141 |         Base::atomicIncrement(this->off[b]);
      |         ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
src/HeterogeneousCore/CUDAUtilities/test/HistoContainer_t.cpp:68:15:   required from 'void go() [with T = short int; int NBINS = 128; int S = 16; int DELTA = 1000]'
   68 |       hr.count(v[j]);
      |       ~~~~~~~~^~~~~~
src/HeterogeneousCore/CUDAUtilities/test/HistoContainer_t.cpp:170:14:   required from here
  170 |   go<int16_t>();
      |   ~~~~~~~~~~~^~
  [src/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h:206](https://github.com/cms-sw/cmssw/blob/CMSSW_15_1_X_2025-02-25-2300/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h#L206):19: warning: casting 'cms::cuda::OneToManyAssoc<unsigned int, 129, -1>::Counter' {aka 'unsigned int'} to 'std::atomic<unsigned int>&' does not use 'constexpr std::atomic<unsigned int>::atomic(__integral_type)' [-Wcast-user-defined]
   206 |         auto &a = (std::atomic<Counter> &)(x);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h: In instantiation of 'static uint32_t cms::cuda::OneToManyAssoc<I, ONES, SIZE>::atomicIncrement(Counter&) [with I = unsigned int; int ONES = 129; int SIZE = 12000; uint32_t = unsigned int; Counter = unsigned int]':
src/HeterogeneousCore/CUDAUtilities/interface/HistoContainer.h:141:30:   required from 'void cms::cuda::HistoContainer<T, NBINS, SIZE, S, I, NHISTS>::count(T) [with T = short int; unsigned int NBINS = 128; int SIZE = 12000; unsigned int S = 16; I = unsigned int; unsigned int NHISTS = 1]'
  141 |         Base::atomicIncrement(this->off[b]);
      |         ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
src/HeterogeneousCore/CUDAUtilities/test/HistoContainer_t.cpp:69:14:   required from 'void go() [with T = short int; int NBINS = 128; int S = 16; int DELTA = 1000]'
   69 |       h.count(v[j]);
      |       ~~~~~~~^~~~~~
src/HeterogeneousCore/CUDAUtilities/test/HistoContainer_t.cpp:170:14:   required from here
  170 |   go<int16_t>();
      |   ~~~~~~~~~~~^~
  [src/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h:206](https://github.com/cms-sw/cmssw/blob/CMSSW_15_1_X_2025-02-25-2300/HeterogeneousCore/CUDAUtilities/interface/OneToManyAssoc.h#L206):19: warning: casting 'cms::cuda::OneToManyAssoc<unsigned int, 129, 12000>::Counter' {aka 'unsigned int'} to 'std::atomic<unsigned int>&' does not use 'constexpr std::atomic<unsigned int>::atomic(__integral_type)' [-Wcast-user-defined]
   206 |         auto &a = (std::atomic<Counter> &)(x);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

[b]

> cat test.cpp
#include <iostream>
#include <atomic>
unsigned int atomicDecrement(unsigned int &x) {
  std::atomic_ref<unsigned int> temp{x};
  return temp--;
}
int main()
{
  unsigned int i=5;
  atomicDecrement(i);
  std::cout <<"i after:"<<i<<std::endl;
  return 0;
}
> test
i after:4

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 26, 2025

cms-bot internal usage

@fwyzard
Copy link
Contributor

fwyzard commented Feb 26, 2025

+1

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47455/43879

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar for master.

It involves the following packages:

  • HeterogeneousCore/CUDAUtilities (heterogeneous)

@cmsbuild can you please review it and eventually sign? Thanks.
@makortel, @missirol, @rovere this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@iarspider
Copy link
Contributor

iarspider commented Feb 26, 2025

See also discussion in #47317

@smuzaffar
Copy link
Contributor Author

please test for el8_amd64_gcc14

@mandrenguyen
Copy link
Contributor

+1

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 20KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11168b/44673/summary.html
COMMIT: 407634b
CMSSW: CMSSW_15_1_X_2025-02-25-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47455/44673/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 10 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3920300
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3920274
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 214 log files, 184 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@cmsbuild cmsbuild merged commit 2b386e4 into cms-sw:master Feb 26, 2025
16 of 17 checks passed
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11168b/44672/summary.html
COMMIT: 407634b
CMSSW: CMSSW_15_1_X_2025-02-25-2300/el8_amd64_gcc14
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47455/44672/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 191 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 100531 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3920300
  • DQMHistoTests: Total failures: 485987
  • DQMHistoTests: Total nulls: 386
  • DQMHistoTests: Total successes: 3433907
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 8.831000000000001 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 10224.0 ): 0.415 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 13034.0 ): 0.938 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 140.045,... ): -0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 141.042 ): 0.043 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 145.014,... ): 0.008 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 145.202 ): 0.023 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 145.408 ): -0.016 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 145.604 ): 0.090 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 145.713 ): -0.008 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 17034.0 ): 3.490 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 250202.181 ): ...
  • Checked 214 log files, 184 edm output root files, 49 DQM output files
  • TriggerResults: found differences in 19 / 47 workflows

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.

5 participants