RFR: 8288904: Incorrect memory ordering in UL [v2]

[Johan Sjölén]

On Wed, 22 Jun 2022 20:36:48 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> I suppose the memory state change due to calling free on a node can reorder to before the load checking there are no readers. The. The memory can appear to be free memory and get allocated to random other memory that changes state arbitrarily, while there is still a concurrent reader.
>
> Right but isn't there a control dependency that the hardware will still obey? Or can the hardware write to memory even if that path is never taken?
> Regarding UB, I could paste the assembly of that instead (maybe I should have done that). My question was whether the cpu can execute that write instruction before even knowing if that branch will be taken.
> Note: I found this interesting article about control dependencies (https://urldefense.com/v3/__https://lwn.net/Articles/860037/__;!!ACWV5N9M2RV99hQ!KJ35L5WteqhN2paAKJd-fNBSSy6UTQMfYkii6nenAdhVtAN4zwcLNDol-4ph7vZvAueJnQE64xlux1FIOG54JI9jcQ$ ). It mentions that the hardware will respect that dependency but there could be some aggressive compiler optimizations on some cases. I don't think that applies here though.
> @fisk sorry, not sure I understood the example.

@pchilano, it seems that it is true that a control dependency establishes that writes are not moved above the read of a control branch (as we do not know which, if any, branch is taken before the read is done). read-on-read allows for moving it up however.

For example:


// OK
x = true;
while(x == true) { }
y = a[0];
~>
x = true;
y = a[0];
while(x == true) {}
// NOT OK
x = true;
while(x == true) { }
a[0] = 5;
~>
x = true;
a[0] = 5;
while(x == true) {}


Source:

https://urldefense.com/v3/__https://www.cl.cam.ac.uk/*pes20/ppc-supplemental/test7.pdf__;fg!!ACWV5N9M2RV99hQ!KJ35L5WteqhN2paAKJd-fNBSSy6UTQMfYkii6nenAdhVtAN4zwcLNDol-4ph7vZvAueJnQE64xlux1FIOG6gNXcOQg$  section 4.2 and 4.4

I believe that that means that this barrier is unnecessary, but it's good manners to do the `Atomic::load`.

Nice catch :-).

-------------

PR: https://git.openjdk.org/jdk/pull/9225