RFR(L): 8193257: PPC64, s390 implementation for Thread-local handshakes
martin.doerr at sap.com
Tue Dec 12 10:07:50 UTC 2017
thanks reviewing. Please see my answers inline.
> I see you enabled it per default?
Yes. I have tried to keep the implementation as close to the other platforms as possible. x86, SPARC and aarch64 have it enabled by default, too.
> Could you add some comment that says what this is doing?
> As it's not doing a safepoint_poll ... its just preparing it, right?
> Maybe also "slow_target" should be "do_safepoint".
I agree with that the label name "do_safepoint" would be more comprehensible, but I'd prefer to use the same name as x86, SPARC and aarch64.
What the function safepoint_poll does is it simply performs the poll by checking the poll bit + conditional branch to the safepoint code. Would you like to see a comment like "Perform poll and branch if safepoint requested"?
> Why do you change memory ordering from release() to lwsync()?
This is not a functional change. release() emits an lwsync() instruction. I have changed it to emphasize that we're using it for acquire and release in one instruction. release() hides that we use it for acquire, too.
> If I understand right you skip the safepoint instruction.
> Why that? Could you please document that this differs
> from the other platforms here?
The s390 code does exactly the same thing as on the other platforms. It adjusts to PC to point to the instruction following the poll instruction. This is part of the new concept which needs to be implemented on all platforms supporting local handshakes. The difference to PPC for example is that s390 has different instruction sizes (2, 4 and 6 bytes) and I implemented it in a way that it works with any poll instruction regardless of its size.
Would you like to see a comment like "Support poll instruction with any size"?
More information about the hotspot-dev