1209873 - IonMonkey: MIPS: Implement memory barrier

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[hev [:hev]]

Implement CodeGenerator::memoryBarrier.

Comment 1

[hev [:hev]]

Attachment: 0001-IonMonkey-MIPS-Add-AssemblerMIPSShared-as_sync.patch

Thanks!

Comment 2

[hev [:hev]]

Attachment: 0002-IonMonkey-MIPS-Implement-memory-barrier.patch

Thanks!

Comment 3

[Tooru Fujisawa [:arai]]

Comment on attachment 8667748 [details] [diff] [review]
0001-IonMonkey-MIPS-Add-AssemblerMIPSShared-as_sync.patch

Review of attachment 8667748 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good :)
(|stype| argument might be unnecessary at this point tho)

Comment 4

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8667749 [details] [diff] [review]
0002-IonMonkey-MIPS-Implement-memory-barrier.patch

Review of attachment 8667749 [details] [diff] [review]:
-----------------------------------------------------------------

I will forward this review to Lars, as he might know better than me.

Comment 5

[Lars T Hansen [:lth]]

Comment on attachment 8667749 [details] [diff] [review]
0002-IonMonkey-MIPS-Implement-memory-barrier.patch

Review of attachment 8667749 [details] [diff] [review]:
-----------------------------------------------------------------

r+, but I'd really like to see as_sync(16) now and a followup bug for other optimizations.

::: js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp
@@ +1507,5 @@
> +void
> +CodeGeneratorMIPSShared::memoryBarrier(MemoryBarrierBits barrier)
> +{
> +    if (barrier)
> +      masm.as_sync();

This is correct as it stands, but:

* It should be as_sync(16), as all you need is an ordering barrier, not a completion barrier, which is potentially much more expensive.

* You probably should also distinguish between the different barrier needs by inspecting the barrierBits.  If the barrier is only LoadLoad then you could emit as_sync(19), cf the MIPS manual and the ARM code.

But that last point is an optimization and it's fine with me if you file a followup bug for "barrier optimization" or something like that, referencing these observations.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/480c75fccf40
https://hg.mozilla.org/integration/mozilla-inbound/rev/c57050c5e021

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/480c75fccf40
https://hg.mozilla.org/mozilla-central/rev/c57050c5e021