On Wed, 9 Dec 2020 09:58:26 GMT, Andrew Haley <[email protected]> wrote:
>> Nick Gasson has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments
>
> src/hotspot/cpu/aarch64/aarch64.ad line 16057:
>
>> 16055:
>> 16056: format %{ "CALL, native $meth" %}
>> 16057:
>
> It might be better to expand this a little to indicate a near or a far call,
> if that's possible.
That would be nice but I can't see how to do it: adlc doesn't allow arbitrary
code inside `format %{ %}`.
> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 3186:
>
>> 3184: // Make sure that native code does not change SVE vector length.
>> 3185: __ verify_sve_vector_length();
>> 3186: }
>
> Do we have to surround every call to verify_sve_vector_length() with if
> (UseSVE > 0) ?
> Why is that check not inside verify_sve_vector_length() ? What is the meaning
> of the
>> 0 comparison? Can it be negative? So many questions. :-)
The valid values are {0,1,2} so > 0 is supposed to read as "SVE1 or SVE2". But
I also think it's neater if the check is inside `verify_sve_vector_length` so
I've moved it there.
> src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 3098:
>
>> 3096: MacroAssembler* masm = _masm;
>> 3097: if (reg->is_Register()) {
>> 3098: __ push(RegSet::of(reg->as_Register()), sp);
>
> Is this right? SP is 16-aligned, so this will use 16 bytes of stack for every
> 8-byte register. Does it get used a lot?
It's used to preserve the native result registers around the runtime calls in
the safepoint and reguard slow paths. With the way the intrinsic works
currently, there'll actually only ever be a single output register. I did it
this way originally to keep the code similar to x86 but actually it's just as
easy to build a `RegSet` directly and push that, so I've removed these
functions.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1711
