StgCmmPrim: Add missing write barrier.
ClosedPublic

Authored by trommler on Sep 11 2016, 3:19 AM.

Details

Summary

On architectures with weak memory consistency a write barrier
is needed before the write to the pointer array.

Fixes Trac #12469

Test Plan

rebuilt Stackage nightly twice on powerpc64le

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
trommler updated this revision to Diff 8703.Sep 11 2016, 3:19 AM
trommler retitled this revision from to StgCmmPrim: Add missing write barrier..
trommler updated this object.
trommler edited the test plan for this revision. (Show Details)
trommler added reviewers: erikd, hvr, simonmar.
trommler updated the Trac tickets for this revision.
erikd accepted this revision.Sep 11 2016, 3:53 AM
erikd edited edge metadata.

How could that even be wrong?

This revision is now accepted and ready to land.Sep 11 2016, 3:53 AM
simonmar requested changes to this revision.Sep 12 2016, 2:57 AM
simonmar edited edge metadata.
simonmar added inline comments.
compiler/codeGen/StgCmmPrim.hs
1359–1361

This line needs to be before mkBasicIndexedWrite, because the purpose is to ensure that the heap writes for the object referred to by val have happened before we write val into the array, and make it visible to other cores. Also add a comment with the ticket number.

Also note that the comment here says "write barrier" but it is talking about a different kind of write barrier: the GC kind.

This revision now requires changes to proceed.Sep 12 2016, 2:57 AM
erikd added a comment.Sep 12 2016, 5:10 AM

I suppose that's why we have more than one person reviewing these things.

Since the placement of that write barrier is so important, I'd love to have some comments there explaining it.

trommler added inline comments.Oct 6 2016, 11:03 AM
compiler/codeGen/StgCmmPrim.hs
1359–1361

Sorry, I only saw your comment today. Phab did not send emails for your comment and @erikd 's second comment.

Issuing a write barrier like you say makes sense to me, but I wonder if the order of the stores done by mkBasicIndexedWrite (above) and emit $ mkstore ... (below) matters. Without the barrier PowerPC would be allowed to reorder those two stores.

Does a GC kind write barrier mean that the instruction above (emit (setInfo...) must be made visible to the garbage collector thread before the store below (emit $ mkStore ...)?

trommler updated this revision to Diff 8962.Oct 10 2016, 5:16 AM
trommler edited edge metadata.
  • Move write barrier and add comment
trommler updated this object.Oct 10 2016, 6:55 AM
trommler edited edge metadata.
trommler marked an inline comment as done.Oct 11 2016, 10:24 AM

Inline comment done.

bgamari accepted this revision.Oct 14 2016, 12:42 PM
bgamari edited edge metadata.

Looks good to me. Thanks @trommler!

This revision was automatically updated to reflect the committed changes.