RFR: 8315954: getArgumentValues002.java fails on Graal [v2]

[Doug Simon]

On Thu, 14 Sep 2023 17:27:35 GMT, Tom Rodriguez <never at openjdk.org> wrote:

>> Doug Simon has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - generalized getLiveObjectLocalsAt to getOopMapAt
>>  - need to zero oop_map_buf
>>  - simplified getLiveObjectLocalsAt and moved it from ResolvedJavaMethod to HotSpotResolvedJavaMethod
>
> src/hotspot/share/jvmci/jvmciCompilerToVM.cpp line 3099:
> 
>> 3097:   JVMCIPrimitiveArray oop_map = JVMCIENV->wrap(oop_map_handle);
>> 3098:   int oop_map_len = JVMCIENV->get_length(oop_map);
>> 3099:   if (nwords > oop_map_len) {
> 
> Should we sanity check against `mask.number_of_entries()`?  One wrinkle here is that `compute_one_oop_map` also computes information about the stack so the mask it computes can be larger than just max_locals.  For the purposes of OSR this doesn't matter as none of the JITs support OSR with a non-empty stack, so we would never call it for a bci with a non-empty stack.  So should we disallow calling it with a non-empty stack or just properly handle it by passing in an array long enough to contain `max_locals + max_stack`?

We only look up the mask for locals and so ignore stack indexes in the mask altogether. I'm assuming therefore that `mask.is_oop(i)` can never hit any problems.
Note that this API should be safe when called for *any* valid BCI, not just those for an OSR entry point. Even if called for a BCI with a non-empty stack, the current implementation simply ignores that part of the mask.

> src/jdk.internal.vm.ci/share/classes/jdk/vm/ci/hotspot/HotSpotResolvedJavaMethodImpl.java line 788:
> 
>> 786: 
>> 787:     @Override
>> 788:     public long getLiveObjectLocalsAt(int bci, BitSet bigOopMap) {
> 
> This seems overly complicated to me.  Why isn't it simply:
> 
>    public BitSet getLiveObjectLocalsAt(int bci) {
>         int locals = getMaxLocals();
>         int nwords = ((locals - 1) / 64) + 1;
>         long liveness[] = new long[nwords];
>         compilerToVM().getLiveObjectLocalsAt(this, bci, liveness);
>         return new BitSet(liveness);
>     }

In the common case we can avoid the overhead of allocating a long array and initializing each of it elements with a JNI upcall.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15705#discussion_r1326318835
PR Review Comment: https://git.openjdk.org/jdk/pull/15705#discussion_r1324860652