RFR: 8164214: [JVMCI] include VarHandle in signature polymorphic method test

[Doug Simon]

> On 22 Aug 2016, at 21:49, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
>> 
>> On 20 Aug 2016, at 00:59, Doug Simon <doug.simon at oracle.com> wrote:
>> 
>> 
>>> On 20 Aug 2016, at 02:26, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>> 
>>> It may be better to expose a new enum value in Flags and set the bit based on the _intrinsic_id, rather than JVMCI checking the range.
>>> 
>>> Thus the Flags enum value for sig-poly remains constant even if the _intrinsic_id values do not (IIUC they will change when new intrinsics are added), thus less needs to be exposed to JVMCI.
>> 
>> Yes, that’s what I was thinking. The intrinsic_id range seems more subject to change than a flag.
>> 
>>> e.g. something like below (not tested)?
>> 
>> In the context of 8164214, I’ve just realised that making it possible to query whether a Method*/HotSpotResolvedJavaMethodImpl is sig-poly is a red herring. In this context, we’re dealing with constant pool resolution so have neither a Method* nor HotSpotResolvedJavaMethodImpl in hand.
>> 
> 
> Drat.
> 
> 
>> Based on this (late - sorry!) realization, I propose to add `String[] CompilerToVM.getSignaturePolymorphicHolderNames()`:
>> 
>> http://cr.openjdk.java.net/~dnsimon/8164214v2
>> 
> 
> I share Christian’s concerns here, as it’s just pushing the hardcoded dependency around.
> 
> Looking more at the sig-poly call i see it already checks the klass names:
> 
>  static bool is_signature_polymorphic_name(Klass* klass, Symbol* name) {
>    return signature_polymorphic_name_id(klass, name) != vmIntrinsics::_none;
>  }
> 
>  vmIntrinsics::ID MethodHandles::signature_polymorphic_name_id(Klass* klass, Symbol* name) {
>    if (klass != NULL &&
>        (klass->name() == vmSymbols::java_lang_invoke_MethodHandle() ||
>         klass->name() == vmSymbols::java_lang_invoke_VarHandle())) {
>      vmIntrinsics::ID iid = signature_polymorphic_name_id(name);
>      if (iid != vmIntrinsics::_none)
>        return iid;
>      if (is_method_handle_invoke_name(klass, name))
>        return vmIntrinsics::_invokeGeneric;
>    }
>    return vmIntrinsics::_none;
>  }
> 
> 
> I guess you want to gate things before calling resolveInvokeHandleInPool?

Yes, we want to avoid VM calls where possible, especially on hot paths like constant pool resolution during bytecode parsing.
> 
> For expediency perhaps it’s best to proceed with the class dependency in JVMCI Java code and then log an issue (noting the JVMCI dependency) to clean up this sig-poly method area properly?

Yes, I was hoping you’d suggest that ;-)

> When i updated this area in HotSpot i took a conservative (and dumb [*]) approach. I found the sig-poly checks quite intertwined and potentially duplicating checks, but i did not wanna wanna mess around with it and start breaking things.

Exactly my reaction when contemplating the more proper solution.

> There are symbol checks for java_lang_invoke_MethodHandle and java_lang_invoke_VarHandle spread a little throughout the code, likewise with system dictionary checks for MethodHandle_klass and VarHandle_klass. I think we could clear all that up as a separate issue.

Sounds good. Thanks for the review.

-Doug