Read Barrier for CompareAndSwapObject #1965
Conversation
Compare-And-Swap on object reference, while primarily is a store operation, it is also an implicit read (it reads the existing value to be compared with a provided compare value, before the store itself), hence needs a read barrier. This change is an extension of eclipse-openj9#1573 with JIT support. Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>
|
@amicic, @dsouzai, @joransiu could I please get a review? And also one of you assigned as a committer for this PR? Note there are also compare and swap instructions generated in |
|
Looks good. Btw, I bet, in real life, very often just a few instructions before this one you'll have a similar one, loading the value to oldVReg (and again very likely, no async check in-between), which would make this one redundant. But I guess, it's hard to prove it. On the other side, the extra cost of this guarded read should not be that high (data would be in L1 already), anyway. Still, I wonder if it would be safe to load the value into the same oldVReg (so that you save on a reg), but I guess it's not - there is possibility that in some cases it would change the semantics of the program. |
I suppose we can write a peephole pass in the future to validate this and remove the redundant read barrier. This is a correctness issue though while a peephole pass would be a performance issue. I'll leave things as is for now.
And the guarded load would not trigger in that case either so yes the load will be as fast as it can be. Thanks for the review! |
|
We should consider adding an NIAI in front of the guarded load, to ensure that the cache line is loaded in the write exclusive state for CAS. |
This work has been completed in #1966. |
Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>
|
Jenkins test sanity |
|
Just a heads up that the Jenkins builds will almost certanly fail because we are waiting for a new level of OMR to be promoted. We may have to re-launch these once this has happened. I can luanch the new builds on your behalf as I'm tracking https://github.com/eclipse/openj9-omr/tree/openj9 like a hawk! |
|
Jenkins test sanity |
|
Windows failures are unrelated infrastructure issues. @joransiu I think we should be good to go here. |
Compare-And-Swap on object reference, while primarily is a store
operation, it is also an implicit read (it reads the existing value to
be compared with a provided compare value, before the store itself),
hence needs a read barrier.
This change is an extension of #1573 with JIT support.
Signed-off-by: Filip Jeremic fjeremic@ca.ibm.com