Up to now, we have not properly respected volatile store semantics on x86.
We are required to ensure that a store is globally visible before we proceed to perform subsequent loads/stores.
It might be a good idea to document what needs to be done before this issue can be closed.
The commit message mentions that the fix should be reviewed and treatment of monitorexit might be incorrect. Are there other things that need to be done?
This fix appears to be working well. A separate JIRA should be created for any other JMM issues.
I think FENCE (rvm/src-generated/opt-ir/OperatorList.dat) was intended to be a full memory fence, so I believe the following fix is needed in rvm/src-generated/opt-ir/OperatorList.dat:
# memory fence
memAsLoad | memAsStore | release
+ # memory fence
+ memAsLoad | memAsStore | release | acquire
FENCE was introduced mainly for implementing volatile accesses. However, volatile store does seem to need to be implemented with a full memory fence (e.g., MFENCE on x86), not just a store fence (e.g., SFENCE on x86). For example: http://gee.cs.oswego.edu/dl/jmm/cookbook.html
Let me know if I should open a new issue and/or do anything else.
Oops, fixing the diff format (not possible to edit/delete comments?):
It should normally be possible to edit your own comments but it's possible that the situation is different for closed issues.
Thanks for the patch. You can leave the opening of a new issue to me. I have some additional (PPC-specific) FENCE-related fixes lying around.