RFR : JDK-8166110 : C2 crash in compiling method handles

[forax at univ-mlv.fr]

Profile information false sharing !

Again, creating a thread local storage of MethodDatas at the starts of methodHandle.invoke so you will get profile info per method handle invoke.
It seems better and more general than by example the adhoc way to profile the condition of a gwt.

Rémi

----- Mail original -----
> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: hotspot-compiler-dev at openjdk.java.net
> Envoyé: Vendredi 3 Février 2017 18:29:56
> Objet: Re: RFR : JDK-8166110 : C2 crash in compiling method handles

> Remi,
> 
>> in the long term, c2 (and c1) should be fixed to stop to inline switch branches
>> that have never been taken,
>> the dynamic language I currently maintain tranforms all switch to a tree of GWTs
>> because of that,
>> i think we will need this improvement to implement pattern matching in 10
>> anyway*.
> 
> Yes, C2 can do better when optimizing switches (e.g., [1]), but
> profiling information is not our friend here. Even if it says all
> branches are equally taken, type paradoxes arise from decisions in the
> context of the current compilation where only one branch is always taken
> at runtime.
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8058192
> 
>> ----- Mail original -----
>>> De: "Vladimir Ivanov" <vladimir.x.ivanov at oracle.com>
>>> À: "Dmitry Chuyko" <dmitry.chuyko at oracle.com>,
>>> hotspot-compiler-dev at openjdk.java.net
>>> Envoyé: Vendredi 3 Février 2017 17:59:53
>>> Objet: Re: RFR : JDK-8166110 : C2 crash in compiling method handles
>>
>>> The fix looks good.
>>>
>>> More detailed description of the problem:
>>>
>>> MH.invokeBasic() and MH.linkTo* signature-polymorphic are inherently
>>> unsafe. There are no type checks involved and it's user responsibility
>>> to ensure there are no type mismatches possible. They are available only
>>> to trusted code (in java.lang.invoke), which ensures the signatures
>>> always match.
>>>
>>> Unfortunately, even though type mismatches aren't possible at runtime,
>>> JIT-compilers can encounter them in paradoxical situations. It usually
>>> happens when optimizing effectively dead code.
>>>
>>> BoundMethodHandle.arg(I) demonstrates one of the problematic code shapes:
>>>
>>> final Object arg(int i) {
>>>     switch (speciesData().fieldType(i)) {
>>>     case L_TYPE: return
>>> speciesData().getters[i].invokeBasic(this);
>>>     case I_TYPE: return (int)
>>> speciesData().getters[i].invokeBasic(this);
>>>     case J_TYPE: return (long)
>>> speciesData().getters[i].invokeBasic(this);
>>>     case F_TYPE: return (float)
>>> speciesData().getters[i].invokeBasic(this);
>>>     case D_TYPE: return (double)
>>> speciesData().getters[i].invokeBasic(this);
>>> }
>>>
>>> If C2 can constant fold speciesData().getters[i], it will try to inline
>>> through invokeBasic() on all execution paths, though only 1 of them is
>>> taken. On all other paths the signatures won't match and it can lead to
>>> type paradoxes on IR level though the code is effectively dead.
>>>
>>> Though it was observed only with invokeBasic() and I don't see such
>>> situations is possible with MH.linkTo*(), for consistency the fix
>>> touches MH linkers as well.
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 2/3/17 5:19 PM, Dmitry Chuyko wrote:
>>>> Summary: some method handles can be used with actual parameters not
>>>> matching their signatures. It's too expensive to check this on call but
>>>> possible to check during inlining. Inlined MHs with wrong usage caused
>>>> further assertion failures in debug builds. The fix adds checks for
>>>> signatures match during MH inlining so it fails if they are not
>>>> matching. New test checks linkToStatic and invokeBasic cases.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8166110
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~vlivanov/dchuyko/8166110/webrev.00/
>>>>
>>>> Testing: new test, Hotspot and JDK tests, promotion benchmarks.
>>>>
>>>> Thanks,
> >>> -Dmitry