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

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

Open
iarspider opened this issue May 28, 2024 · 13 comments
Open

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

iarspider opened this issue May 28, 2024 · 13 comments

Comments

@iarspider
Copy link
Contributor

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

src/DataFormats/Math/test/FastMath_t.cpp: In instantiation of 'void {anonymous}::sampleSquare() [with T = float]':
src/DataFormats/Math/test/FastMath_t.cpp:108:22:   required from here
  src/DataFormats/Math/test/FastMath_t.cpp:71:23: warning: compound assignment with 'volatile'-qualified left operand is deprecated [-Wvolatile]
    71 |                 dummy += yy;  // add a bit of random instruction
      |                 ~~~~~~^~~~~
src/DataFormats/Math/test/FastMath_t.cpp: In instantiation of 'void {anonymous}::sampleSquare() [with T = double]':
src/DataFormats/Math/test/FastMath_t.cpp:109:23:   required from here
  src/DataFormats/Math/test/FastMath_t.cpp:71:23: warning: compound assignment with 'volatile'-qualified left operand is deprecated [-Wvolatile]
<...>
In file included from src/DataFormats/Math/test/testAtan2.cpp:9:
  src/DataFormats/Math/test/rdtscp.h:31:10: warning: 'volatile'-qualified return type is deprecated [-Wvolatile]
    31 |   inline volatile unsigned long long rdtsc() { return __rdtscp(&rdtscp_val); }
      |          ^~~~~~~~
<...>
In file included from src/DataFormats/Math/test/testAtan2.cpp:9:
  src/DataFormats/Math/test/rdtscp.h:31:10: warning: 'volatile'-qualified return type is deprecated [-Wvolatile]
    31 |   inline volatile unsigned long long rdtsc() { return __rdtscp(&rdtscp_val); }
      |          ^~~~~~~~
<...>
In file included from src/DataFormats/Math/test/testAtan2.cpp:9:
  src/DataFormats/Math/test/rdtscp.h:31:10: warning: 'volatile'-qualified return type is deprecated [-Wvolatile]
    31 |   inline volatile unsigned long long rdtsc() { return __rdtscp(&rdtscp_val); }
      |          ^~~~~~~~
<...>
In file included from src/DataFormats/Math/test/testApproxMath.cpp:137:
  src/DataFormats/Math/test/rdtscp.h:31:10: warning: 'volatile'-qualified return type is deprecated [-Wvolatile]
    31 |   inline volatile unsigned long long rdtsc() { return __rdtscp(&rdtscp_val); }
      |          ^~~~~~~~
src/CondCore/Utilities/bin/conddb_test_gt_perf.cpp: In member function 'void FetchWorker::runFake()':
  src/CondCore/Utilities/bin/conddb_test_gt_perf.cpp:360:20: warning: '++' expression of 'volatile'-qualified type is deprecated [-Wvolatile]
   360 |   void runFake() { fooGlobal++; }
      |                    ^~~~~~~~~
src/CondCore/Utilities/bin/conddb_test_gt_perf.cpp: In member function 'void DeserialWorker::runFake()':
  src/CondCore/Utilities/bin/conddb_test_gt_perf.cpp:406:20: warning: '++' expression of 'volatile'-qualified type is deprecated [-Wvolatile]
   406 |   void runFake() { fooGlobal++; }
      |                    ^~~~~~~~~
@iarspider
Copy link
Contributor Author

assign DataFormats/Math, CondCore/Utilities

@cmsbuild
Copy link
Contributor

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

cms-bot internal usage

@cmsbuild
Copy link
Contributor

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

iarspider commented Jun 18, 2024

@fwyzard
Copy link
Contributor

fwyzard commented Jun 18, 2024

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

  • rewrite the code from
    co[i] += ws[warpId - 1];  // line 78
    ...
    co[i] += psum[k];         // line 183
    to
    co[i] = co[i] + ws[warpId - 1];  // line 78
    ...
    co[i] = co[i] + psum[k];         // line 183
    which honestly is just stupid
  • or disable the warning (hint hint)

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

@iarspider
Copy link
Contributor Author

or disable the warning (hint hint)

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

@fwyzard
Copy link
Contributor

fwyzard commented Jun 18, 2024

So we will care about it in 2030.

@fwyzard
Copy link
Contributor

fwyzard commented Jun 18, 2024

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

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

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

@jfernan2
Copy link
Contributor

I see that 14_2_X does not have yet the -diag-suppress
Is there anybody against it?

@fwyzard
Copy link
Contributor

fwyzard commented Oct 30, 2024

No objections, but wasn't cms-sw/cmsdist#9252 already merged in 14.1.x ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants