[9] RFR (L): 8072008: Emit direct call instead of linkTo* for recursive indy/MH.invoke* calls

[Vladimir Ivanov]

No, it's not needed. All relocations refer to recorded metadata section 
in the nmethod:

+ Method* virtual_call_Relocation::method_value() {
+   Metadata* m = code()->metadata_at(_method_index);
+   assert(m != NULL || _method_index == 0, "should be non-null for 
non-zero index");
+   assert(m == NULL || m->is_method(), "not a method");
+   return (Method*)m;
+ }

The following code in nmethod::metadata_do should enumerate them:

   // Visit the metadata section
   for (Metadata** p = metadata_begin(); p < metadata_end(); p++) {
     if (*p == Universe::non_oop_word() || *p == NULL)  continue;  // 
skip non-oops
     Metadata* md = *p;
     f(md);
   }

Special cases are needed for embedded metadata.

And recording happens in resolved_method_index (see *.ad changes):

src/share/vm/opto/machnode.hpp:
+   int resolved_method_index(CodeBuffer &cbuf) const {
+     if (_override_symbolic_info) {
+       // Attach corresponding Method* to the call site, so VM can use 
it during resolution
+       // instead of querying symbolic info from bytecode.
+       assert(_method != NULL, "method should be set");
+       assert(_method->constant_encoding()->is_method(), "should point 
to a Method");
+       return 
cbuf.oop_recorder()->find_index(_method->constant_encoding());
+     }
+     return 0; // Use symbolic info from bytecode (resolved_method == 
NULL).
+   }
Best regards,
Vladimir Ivanov

On 10/30/15 10:46 PM, Dean Long wrote:
> Do you need *_call_Relocation::method_value() to be scanned in any new
> places,
> like nmethod::metadata_do?
>
> dl
>
> On 10/29/2015 2:36 PM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8072008/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8072008
>>
>> NB! It touches aarch64 & ppc code, so I'd like to hear from the
>> maintainers whether the changes are fine.
>>
>> To simplify review process, the changes can be split in 2 parts.
>>
>> http://cr.openjdk.java.net/~vlivanov/8072008/webrev.01/hotspot.reloc/
>>
>> Platform-dependent changes in macro assembler to switch from
>> relocation types to relocation instances, and attach pre-resolved
>> method to call sites in generated code.
>>
>> http://cr.openjdk.java.net/~vlivanov/8072008/webrev.01/hotspot.attach/
>> High-level adjustments in C2 and test cases.
>>
>> Overview
>>
>> There's a performance problem with inlining through
>> MH.linkTo*/invokeBasic linkers. When MemberName/receiver is constant
>> and VM sees the target method during compilation, but decides not to
>> inline (e.g. recursive inlining), a call to linker is generated, but
>> more optimal code shape is to call target method directly.
>>
>> The problem with substituting a linker call by a direct/virtual call
>> is that call site resolution logic relies on bytecode info (see
>> SharedRuntime::resolve_{static,opt_virtual,virtual}_call_C and
>> SharedRuntime::find_callee_info_helper). But on bytecode level
>> symbolic reference points to the linker method, which doesn't tell
>> anything about target method.
>>
>> The fix is to attach additional metadata (Method*) to the call site,
>> so VM can use it instead of inspecting bytecode when linking the call
>> site. The Method* is placed in relocation table (for static_call,
>> opt_virtual_call, and virtual_call relocation types) and contains a
>> Method* the call site is resolved to by JIT compiler (_method from a
>> call IR node).
>>
>> It is used only in C2 generated code.
>>
>> Example:
>>
>>   MemberName mn = ... <<T.f1>> ... ; // compile-time constant
>>   MethodHandle.linkToStatic(..., mn)
>>
>> compiles to:
>>
>>   callq  0x0000000109cb1900  ; OopMap{off=28}
>>                              ; *invokestatic linkToStatic
>>                              ; - Test::invokeStatic at 3 (line 108)
>>                              ; {static_call T.f1}
>>
>> It's a call to MethodHandle.linkToStatic on bytecode level, but in
>> generated code it's a direct call to T.f1.
>>
>> For testing purposes, 2 new WhiteBox methods are introduced:
>>   - clearInlineCaches
>>   - deoptimize
>>
>> They are used in unit tests (test/compiler/jsr292/NonInlinedCall/*)
>> which stresses new code shapes.
>>
>> WhiteBox changes require some adjustments in build scripts, since
>> @HotSpotIntrinsicCandidate isn't part of JDK8. But it will be fixed
>> separately (the idea is to start building wb.jar with JDK9).
>>
>> Testing: JPRT, java/lang/invoke, nashorn, octane
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>