JDK-8217998: Remove method_type field associated with the appendix field of an indy or method handle call

[Lois Foltan]

On 2/4/2019 11:53 AM, Karen Kinnear wrote:
> Lois,
>
> Code looks good. Couple of minor questions.comments
Thank you Karen for the review!  Addressed all your comments below 
please see additional responses.  Updated webrev at:

http://cr.openjdk.java.net/~lfoltan/bug_jdk8217998.2/webrev/

Rerunning hs-tier1-3 & jdk-tier1-3 testing.

>
> 1. rewriter.hpp: comment line 162 needs updating - we just use 1 entry now
Fixed the comment and also renamed the method from 
"add_invokedynamic_resolved_references_entries" to 
"add_invokedynamic_resolved_references_entry".  We were always only 
adding 1 entry even though previously that entry spanned multiple slots 
in the _invokedynamic_references_map.

>
> 2. jvmciCompilerToVM.cpp
> You removed an assertion about method_type_if_resolved.
> Would it make sense to add an assertion that !has_local_sig - which if I am reading this correctly,
> you are setting to indicate sig-poly methods?
Added.

>
> Does jvmci use the method_type field?  Or was removing this assertion the only change needed?
This was the only instance of the use.  However, I have sent email to 
Dean Long and cc'd Vladimir Kozlov asking for review.

>
> 3. ciStreams.cpp
> comment line 367: “There is are” -> “There are”
Fixed.

>
> 4. ciStreams.cpp
> line 336 sets local_signature
> Would it make sense to only do that inside the check for if (has_local_signature()) - e.g. after line 393?
Fixed.

>
> 5. Testing - the comments in ciStreams.cpp get_method are very helpful
> Are there existing tests for these cases or do we need any new ones?
I did check the tests within jdk/java/lang/invoke, numerous scenarios 
for VH.get(), Lambda form for a method call dealing with linkStatic as 
the comments indicate, and tests dealing with variable arity, etc.  So I 
believe the cases are covered.

Thanks,
Lois

>
> thanks,
> Karen
>
>> On Feb 4, 2019, at 7:32 AM, Lois Foltan <lois.foltan at oracle.com> wrote:
>>
>> Please review this change that removes the unused method_type field associated with the appendix field of an indy or method handle call.  This is the first of a series of changes working towards JDK-8210685: Bootstrap method consolidation.
>>
>> open webrev at: http://cr.openjdk.java.net/~lfoltan/bug_jdk8217998.1/webrev/
>> bug link: https://bugs.openjdk.java.net/browse/JDK-8217998
>> co-contributors: Lois Foltan & John Rose
>>
>> Testing: hs-tier1-4 & jdk-tier1-3 (all platforms), hs-tier5-8 (linux only), JCK vm & lang (linux only)
>>
>> Thanks,
>> Lois