Hi Ningsheng,

On 2020-08-24 11:59, Ningsheng Jian wrote:
Hi Erik,

Thanks for the review!

On 8/22/20 12:21 AM, Erik Österlund wrote:
Hi,

Have you tried this with ZGC on AArch64? It has custom code for saving
live registers in the load barrier slow path.
I can't see any code changes there, so assuming this will just crash
instead.
The relevant code is in ZBarrierSetAssembler on aarch64.

Maybe I missed something?


I didn't add ZGC option while running tests. I think I need to update push_fp() which is called by ZSaveLiveRegisters. But do we need to get size info (float/neon/sve) instead of saving the whole vector register? Currently, it just simply saves the whole NEON register.

What we found on x86_64 was that there was a significant cost in saving vector registers in load barriers. That is why we perform some analysis so that only the exact registers that are affected, and only the parts of the registers that are affected, get spilled. It actually mattered. It will of course work either way, but that was our observation on x86_64. But I am okay with that being deferred to a separate RFE. I just wanted to make sure that it at the very least works with the new code, for a start, so it doesn't start crashing.

And in ZBarrierSetAssembler::load_at(), before calling to runtime code, we call push_call_clobbered_registers_except(), which just saves floating point registers instead of the whole NEON vector registers. Similar behavior in x86 implementation. Is that correct (not saving vectors)?

Yes. The call contexts are:
1) Interpreter. Does not use vector registers.
2) Method handle intrinsic. Uses only floats that are part of the Java calling convention, rest is garbage. No vectors here.
3) Checkcast arraycopy. Does not use vectors.

Thanks,
/Erik

Thanks,
Ningsheng

Reply via email to