RFR (XS): 8224110 [lworld] Reflective method invocation fails with assert: "illegal type"

[David Simms]

Update webrev: http://cr.openjdk.java.net/~dsimms/valhalla/8224110/

Bug: https://bugs.openjdk.java.net/browse/JDK-8224110

So I've updated the webrev, found two uses "jobjectArray" passed into 
the JVM from user code, where a inline type array may be in used as 
varargs: "JVM_InvokeMethod" and "JVM_NewInstanceFromConstructor".

Added a helper method for converting to objArray to keep the rest of the 
VM code happy.

/D

On 2019-06-13 08:48, David Simms wrote:
>
> Thanks for pointing out this is a more general issue...your counter 
> use case is useful.
>
> Will have a look around for other interesting cases. For LW2 there 
> might be an initial hack to convert to objArrayOop.
>
> There is an issue in the VM now where, yes java inline type arrays are 
> co-variant but objArrayOop and valueArrayOop do not share the same 
> type relationship, and maybe they should. I.e. VM arrayOop get/put 
> elements methods (yeah there is polymorphism in C++). Unclear we would 
> win anything in terms of performance or heap for VM runtime code. VM 
> runtime always handles oops (drastically reduces code changes), 
> meaning conversion to objArrayOop is actually fine.
>
> JIT intrinsics will benefit if we allow inline type array.
>
> I'll rework the patch, and do a little more digging. Thanks for the 
> feedback guys.
>
> /D
>
> On 12/06/2019 21:26, Roger Riggs wrote:
>> Hi David,
>>
>> Is that unique to j.l.invoke?
>> There are plenty uses of varargs where the arguments
>> are the same type. <T> List.of(<T>... strings), for example.
>> There is the ambiguity for a single argument that the compiler warns 
>> about
>> as in the example that developers have to deal with.
>>
>> We should avoid exceptions, they require more specification (and warts)
>> than being able to just use the whole spec.
>> Getting arguments to this kind of method or constructor efficiently 
>> is important.
>>
>> $.02, Roger
>>
>>
>> On 6/12/19 12:53 PM, David Simms wrote:
>>>
>>> Updated the bug with comment, I was about say, sure let's use inline 
>>> type array if you want. But the use cases are thin:
>>>
>>> "The more I think about it the less I think we should allow inline 
>>> type array, not useless all the method args the same inline type, ie:
>>>
>>> "test(MyValue3 v1, MyValue3 v2, MyValue3 v3)", then passing 
>>> MyValue3[] would make sense...but only when all the arg types are 
>>> the same...seems like an edge case"
>>>
>>> All the parameter types have to be the same...is that worth a 
>>> special case ?
>>>
>>>
>>> On 12/06/2019 16:52, Roger Riggs wrote:
>>>> Hi,
>>>>
>>>> Would this be just a temporary restriction?
>>>> I could see the benefits of being able to use an inline array class 
>>>> for varargs.
>>>>
>>>> ?, Roger
>>>>
>>>> On 06/12/2019 09:27 AM, David Simms wrote:
>>>>> Greetings,
>>>>>
>>>>> Please review this small fix to prevent "inline type" arrays being 
>>>>> sent directly "JVM_InvokeMethod" and 
>>>>> "JVM_NewInstanceFromConstructor". I don't believe it is the 
>>>>> intention of these methods to receive such a type, in the original 
>>>>> problem, there was a javac warning of passing arg[0] implicitly 
>>>>> converted to args array.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8224110
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~dsimms/valhalla/8224110/
>>>>>
>>>>>
>>>>
>>