[15] RFR(XS): 8239083: C1 assert(known_holder == NULL || (known_holder->is_instance_klass() && (!known_holder->is_interface() || ((ciInstanceKlass*)known_holder)->has_nonstatic_concrete_methods())), "should be non-static concrete method");

[Tobias Hartmann]

Oops, pressed reply on the wrong email :D

Best regards,
Tobias


On 02.06.20 08:30, Tobias Hartmann wrote:
> Thanks Vladimir!
> 
> Best regards,
> Tobias
> 
> On 02.06.20 04:09, Vladimir Kozlov wrote:
>> +1
>>
>> Thanks,
>> Vladimir
>>
>> On 5/26/20 6:56 AM, Tobias Hartmann wrote:
>>> Hi Christian,
>>>
>>> looks reasonable to me.
>>>
>>> Best regards,
>>> Tobias
>>>
>>> On 15.05.20 11:11, Christian Hagedorn wrote:
>>>> Hi
>>>>
>>>> Please review the following patch:
>>>> https://bugs.openjdk.java.net/browse/JDK-8239083
>>>> http://cr.openjdk.java.net/~chagedorn/8239083/webrev.00/
>>>>
>>>> The assert fails in the test case when invoking the only static interface method with a method
>>>> handle. In this case, known_holder is non-NULL. However, known_holder would be set to NULL at [1]
>>>> since the call returns NULL when known_holder is an interface.
>>>>
>>>> In the failing test case, known_holder is non-NULL since GraphBuilder::try_method_handle_inline()
>>>> calls GraphBuilder::try_inline() with holder_known set to true which eventually lets profile_call()
>>>> to be called with a non-NULL known_holder argument.
>>>>
>>>> On the other hand, when calling a static method without a method handle, known_holder seems to be
>>>> always NULL:
>>>> profile_call() is called directly at [2] with NULL or indirectly via try_inline() [3]. In the latter
>>>> case, cha_monomorphic_target and exact_target are always NULL for static methods and therefore
>>>> known_holder will also be always NULL in profile_call().
>>>>
>>>> We could therefore just remove the assert which seems to be too strong (not handling this edge
>>>> case). Another option would be to change the call to try_inline() in try_method_handle_inline() to
>>>> only set holder_known to true if the target is not static. The known_holder is eventually only used
>>>> in LIR_Assembler::emit_profile_call() [4] but only if op->should_profile_receiver_type() holds [5].
>>>> This is only true if the callee is not static [6]. The webrev uses the second approach.
>>>>
>>>> What do you think?
>>>>
>>>> Best regards,
>>>> Christian
>>>>
>>>>
>>>> [1]
>>>> http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/share/c1/c1_GraphBuilder.cpp#l4386
>>>> [2]
>>>> http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/share/c1/c1_GraphBuilder.cpp#l3571
>>>> [3]
>>>> http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/share/c1/c1_GraphBuilder.cpp#l2039
>>>> [4]
>>>> http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#l3589
>>>>
>>>> [5]
>>>> http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp#l3584
>>>>
>>>> [6] http://hg.openjdk.java.net/jdk/jdk/file/dd0caf00b05c/src/hotspot/share/c1/c1_LIR.hpp#l1916