Fix for a missing allocationFence in transformArrayCloneCall() #7117
Conversation
| // Flush after the arraycopy so that the cloned array appears identical to the original before it's made | ||
| // visible to other threads, and because the obj allocation is allowed to use non-zeroed TLH, we need to | ||
| // make sure no thread sees stale memory contents from the array element section. | ||
| callTree->insertBefore(TR::TreeTop::create(comp(), allocationFence)); |
There was a problem hiding this comment.
Are allocation fence nodes generated in general after allocations on platforms such as X86, that do not have weak memory model ? I ask, because this insertion of an allocation fence seems to be unconditional in that sense, it will add it across platforms. This may not be incorrect, but I thought I'd check if it was unconventional (e.g. if no one code ever generated an allocation fence on non-weak ordered platforms) since that could lead to bugs in its own way (i.e. IL that is unexpectedly different).
There was a problem hiding this comment.
Yes, good catch.. I had looked at this but somehow missed the cg()->getEnforceStoreOrder() check in the genFence() routine used in ILGen. I added that check now.
When we transform a clone() of an array object into a newarray and arraycopy, we neglected to add an allocationFence. This change will introduce an allocationFence after the arraycopy to ensure that all threads see the new contents of memory on weak memory coherency machines like POWER and AArch64. Signed-off-by: Kevin Langman <langman@ca.ibm.com>
klangman
force-pushed
the
fix-missing-AF-for-array-clone-transform
branch
from
eecbc70
to
d14f6e3
Compare
|
jenkins build all |
|
Fix looks simple/good to me. Checks have passed. Merging. |
When we transform a clone() of an array object into a newarray and arraycopy, we neglected to add an allocationFence. This change will introduce an allocationFence after the arraycopy to ensure that all threads see the new contents of memory on weak memory coherency machines like POWER and AArch64.