On 10/15/22 09:26, Jin-oh Kang wrote:
On Wed, Oct 12, 2022 at 3:08 AM Zebediah Figura zfigura@codeweavers.com wrote:
On 10/3/22 07:45, Jinoh Kang wrote:
@@ -5326,14 +5326,17 @@ BOOL WINAPI NtUserSetProcessDpiAwarenessContext( ULONG awareness, ULONG unknown */ ULONG WINAPI NtUserGetProcessDpiAwarenessContext( HANDLE process ) {
- DPI_AWARENESS val;
if (process && process != GetCurrentProcess()) { WARN( "not supported on other process %p\n", process ); return NTUSER_DPI_UNAWARE; }
- if (!dpi_awareness) return NTUSER_DPI_UNAWARE;
- return dpi_awareness;
- val = ReadAcquire( &dpi_awareness );
- if (!val) return NTUSER_DPI_UNAWARE;
- return val;
Why does this need acquire semantics?
It was intended to ensure that loads by two successive NtUserGetProcessDpiAwarenessContext() calls don't get reordered with each other.
If you think this won't be a problem (i.e. such reordering cannot actually happen in C++ memory model, or the user is responsible for serializing access), I'll downgrade it to ReadNoFence.
According to my reading this is prohibited most particularly by § 5.1.2.4.22 of the C11 (draft) specification:
"Furthermore, if a value computation A of an atomic object M happens before a value computation B of M, and the value computed by A corresponds to the value stored by side effect X, then the value computed by B shall either equal the value computed by A, or be the value stored by side effect Y, where Y follows X in the modification order of M."
The following note also suggests that this reading is the correct one:
"This effectively disallows compiler reordering of atomic operations to a single object, even if both operations are ‘‘relaxed’’ loads."
The prototypical use of acquire/release is to protect a *different* variable, which may or may not be atomic. Note also that an acquire with no corresponding release is highly suspicious.