Use _mm_pause, load loop #1146
Use _mm_pause, load loop #1146
Conversation
Don't consume _CRT_SATELLITE_1, it is not ours
# Conflicts: # tests/std/test.lst
# Conflicts: # tests/std/test.lst
Co-Authored-By: Stephan T. Lavavej <stl@nuwen.net>
yes, non-specific Co-Authored-By: Stephan T. Lavavej <stl@nuwen.net>
Also constexpr atomic initializers
Co-authored-by: Billy O'Neal <bion@microsoft.com>
BillyONeal
left a comment
BillyONeal
left a comment
There was a problem hiding this comment.
Approving despite a desire to not mention the optimization manual to dismiss my waiting status.
|
I've finally heard back from Intel concerning the code snippet from the manual. The code in the manual is licensed under the 0BSD license. This means we don't need to provide any attribution, license, or copyright text. Our lawyers have suggested just keeping the comment text as-is, additionally noting the 0BSD license (nothing formal is needed). |
# Conflicts: # tests/std/include/test_atomic_wait.hpp
|
Updated. There's still GH-1197 |
CaseyCarter
left a comment
CaseyCarter
left a comment
There was a problem hiding this comment.
I've "requested changes" here which I'll apply myself after the fix for GH-1197 is applied internally (intrin.h and intrin0.h are part of vcruntime which is not yet opensource).
Co-authored-by: Casey Carter <cartec69@gmail.com>
CaseyCarter
left a comment
CaseyCarter
left a comment
There was a problem hiding this comment.
I've pushed a change to (1) define _mm_pause directly in xatomic.h during the transition until the vcruntime change to intrin0.h goes live, and (2) Do The Right Thing in ARM64EC mode.
|
I see that everything looks complete now |
Thanks for the reminder: I've resolved all open comments and moved this back to Final Review. |
| // Example 2-4. Contended Locks with Increasing Back-off Example - Improved Version, page 2-22 | ||
| // The code in mentioned manual is covered by the 0BSD license. | ||
| int _Current_backoff = 1; | ||
| const int _Max_backoff = 64; |
There was a problem hiding this comment.
I observe that this could be constexpr but it doesn't make a difference - no change requested.
| // timing assumption that the main thread evaluates the `wait(old_value)` before this timeout expires | ||
| std::this_thread::sleep_for(waiting_duration); | ||
| add_seq('6'); | ||
| #endif // CAN_FAIL_ON_TIMING_ASSUMPTION |
There was a problem hiding this comment.
I'm not sure why this is being removed (more than 1 line is guarded) but as there are no nearby preprocessor guards, no change requested.
|
Thanks for this performance improvement! This PR went through quite a pause but it should be worth the wait. ⏸️ 😹 |
5 participants
Resolves #680