[8] RFR (XS): 8031695: CHA ignores default methods during analysis leading to incorrect code generation

[Vladimir Ivanov]

Harold,

I'm reluctant to change it with this fix.

Dependencies::is_concrete_method(ciMethod* m) isn't called from anywhere 
and it already diverged a bit from Method* version. Also, method type 
(normal or overpass) isn't cached in ciMethod, so is_overpass() isn't 
available unless corresponding Method is queries instead.

At least, it needs additional investigation. Probably, this function 
should go away or delegate to Method* version.

Thanks for looking into the fix!

Best regards,
Vladimir Ivanov

On 1/22/14 11:40 PM, harold seigel wrote:
> Hi Vladimir.
>
> Does this second version of is_concrete_method() also need a "&&
> !m->is_overpass()"?
>
> 1191 bool Dependencies::is_concrete_method(ciMethod* m) {
> 1192   // Statics are irrelevant to virtual call sites.
> 1193   if (m->is_static())  return false;
> 1194
> 1195   // We could also return false if m does not yet appear to be
> 1196   // executed, if the VM version supports this distinction also.
> 1197   return !m->is_abstract();
> 1198 }
>
> Thanks, Harold
>
>
> On 1/22/2014 12:29 PM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8031695/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8031695
>>
>> CHA erroneously ignores default methods when looking for different
>> versions of a method in a class hierarchy.
>>
>> Consider the following hierarchy:
>>   interface I { default void m() { ... }}
>>   class A implements I {}
>>
>>   class C extends A { }
>>   class D extends A { public void m() { ... } }
>>
>> Call site: invokevirtual A.m()
>>
>> A.m resolves to I.m. Searching from A, CHA finds only D.m and
>> concludes that D.m is the only implementation of I.m, since
>> Dependencies::is_concrete_method returns false for I.m.
>>
>> But there is C with default implementation from I. So, if VM inlines
>> D.m and then C's instance is passed as objectref, the wrong method is
>> invoked (D.m instead of I.m).
>>
>> The fix is to take default methods into account doing CHA analysis and
>> treat the call site as non-monomorphic when a concrete method is found
>> for a default method.
>>
>> Conservative approach is chosen, since the fix is targeted for 8.
>>
>> Testing: failing application, regression test, vm.defmeth.testlist,
>> JDK regression tests (java/util/, java/lang)
>>
>> Thanks!
>>
>> Best regards,
>> Vladimir Ivanov
>