1715494 - Use dmb ish, not dmb ishst, as beforeStore fence for atomic ops

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Lars T Hansen [:lth]]

(Spun off from bug 1714367)

For a plain "atomic store 37" in isolation we have eg this (Ion code):

<alignment guard>
0x268f321d2040  528004a1  mov     w1, #0x25
0x268f321d2044  d5033abf  dmb     ishst
0x268f321d2048  b8206aa1  str     w1, [x21, x0]
0x268f321d204c  d5033bbf  dmb     ish

dmb ishst is slightly weaker than just a dmb ish. The intent of the dmb ishst; str; dmb ish is to implement a store-release operation, which is what we really want here (see bug 1442534, but that's for later). The question is, is this the correct implementation, or should the first fence also be dmb ish?

After some searching I have concluded that "it's complicated". The OpenJDK HotSpot compiler used to have the same code as we do, with dmb ishst before a volatile store and a dmb ish after. In this paper (page "19", near top) it is on the one hand stated that this is "overly weak" for volatile semantics; in the same paragraph it is then stated that it is nevertheless "sufficient" for reasons that are essentially the same as ours, namely, every op ends with a dmb ish anyway and establishes the necessary relation; and a footnote explains that the HotSpot maintainers accepted a change that changed the ishst to an ish (see here and here). Maybe the confusion stems from two separate products being mentioned, standard JVM HotSpot and a volatile-all-the-things research project, which has slightly different semantics.

In short, I think that the code we have is probably correct, but since there is only the tiniest edge to be gained from this we should change our codegen so that there's no doubt, until we can switch to the better ARM64 instructions.

Comment 1

[Lars T Hansen [:lth]]

Attachment: Bug 1715494 - Simplify ARM64 barrier logic. r?jseward

Just emit dmb ish for every barrier, do not try to use more
discriminating barriers such as dmb ishst as they are hard to reason
about.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0003f0a91f46
Simplify ARM64 barrier logic. r=jseward

Comment 3

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/0003f0a91f46