cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.07k stars 4.29k forks source link

C++20 and deprecation some uses of `volatile` #45072

Open iarspider opened 4 months ago

iarspider commented 4 months ago

In C++20, many uses of volatile keyword are deprecated, see Proposal 1152. This creates warnings in C++20 IBs:

iarspider commented 4 months ago

assign DataFormats/Math, CondCore/Utilities

cmsbuild commented 4 months ago

New categories assigned: reconstruction,db

@jfernan2,@mandrenguyen,@francescobrivio,@saumyaphor4252,@perrotta,@consuegs you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild commented 4 months ago

cms-bot internal usage

cmsbuild commented 4 months ago

A new Issue was created by @iarspider.

@antoniovilela, @Dr15Jones, @smuzaffar, @makortel, @rappoccio, @sextonkennedy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

iarspider commented 3 months ago

Now that we capture CUDA warnings, there are some more uses of volatile reported: https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc12/CMSSW_14_1_CPP20_X_2024-06-14-1100/HeterogeneousCore/CUDAUtilities, https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc12/CMSSW_14_1_CPP20_X_2024-06-14-1100/CUDADataFormats/TrackingRecHit . @cms-sw/heterogeneous-l2 could you please check?

fwyzard commented 3 months ago

According to the CUDA documentation:

The compiler is free to optimize reads and writes to global or shared memory (for example, by caching global reads into registers or L1 cache) as long as it respects the memory ordering semantics of memory fence functions (Memory Fence Functions) and memory visibility semantics of synchronization functions (Synchronization Functions).

These optimizations can be disabled using the volatile keyword: If a variable located in global or shared memory is declared as volatile, the compiler assumes that its value can be changed or used at any time by another thread and therefore any reference to this variable compiles to an actual memory read or write instruction.

So, if volatile was needed in the CUDA code, the options are

Interestingly, the alpaka version does not use volatile. So now I'm concerned that it may be incorrect :-/

iarspider commented 3 months ago

or disable the warning (hint hint)

volatile compound-assignment statements are planned to be removed in C++26: P2866R0

fwyzard commented 3 months ago

So we will care about it in 2030.

fwyzard commented 3 months ago

volatile compound-assignment statements are planned to be removed in C++26: P2866R0

Actually, are you sure ?

Following the C++20 deprecations, the C committee looked to adopt a similar stance on volatile and were given feedback that a number of vendors were strongly opposed to the deprecation of compound-assignment operators, as among other reasons, many hardware APIs and device drivers would expect to use volatile compound assignment to communicate with their devices. This subset of the deprecated functionality was undeprecated for C++23 by [P2327R1], followed by further undeprecations in [CWG2654]

CWG2654 says that compound-assignment statements have been un-deprecated in C++ 23.

iarspider commented 3 months ago

Contrary to the direction desired in the NB comment, EWG resolved to un-deprecate all volatile compound assignments

I've read that they only decided to un-deprecate bitwise compound operators. Then yes, we can silence this warning.

smuzaffar commented 3 months ago

so as suggested by the compiler

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

we can add -diag-suppress 3012 in https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_14_1_X/master/scram-tools.file/tools/cuda/cuda.xml#L14 to suppress this