RFR 8233920: MethodHandles::tryFinally generates illegal bytecode for long/double return types

[Remi Forax]

Hi Jorn, Claes,
Is it not better to always store in a local variable, like javac does, instead of doing the double SWAP if there is a return value of size 1 ?

Rémi 

----- Mail original -----
> De: "Jorn Vernee" <jorn.vernee at oracle.com>
> À: "Claes Redestad" <claes.redestad at oracle.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Mardi 12 Novembre 2019 15:56:06
> Objet: Re: RFR 8233920: MethodHandles::tryFinally generates illegal bytecode for long/double return types

> Hi Claes,
> 
> Thanks for the comments!
> 
> Updated webrev: http://cr.openjdk.java.net/~jvernee/8233920/webrev.02/
> 
> Also, thanks for sponsoring.
> 
> Jorn
> 
> On 12/11/2019 15:30, Claes Redestad wrote:
>> Hi Jorn,
>>
>> good catch!
>>
>> Some minor stylistic concerns:
>>
>> "* = depends on whether the return type is a category 2 type, which
>> takes up 2 stack slots."
>>
>> While long/double are known to use two stack slots, "category 2 type"
>> isn't used much outside the specification. I'd either simplify to
>> "* = depends on whether the return type takes up 1 or 2 stack slots."
>> or add a reference to the definition of type categories.
>>
>> Similarly I think:
>>
>> boolean returnsCat2Type = returnType == long.class || returnType ==
>> double.class;
>>
>> could be simplified as:
>>
>> boolean isDoubleWord = basicReturnType.isDoubleWord(); //
>> returnsDoubleWord?
>>
>> In the test, is it possible to narrow the expected exception to the
>> exact type being thrown (T1?) rather than Throwable?
>>
>> I can sponsor the patch.
>>
>> /Claes
>>
>> On 2019-11-12 12:09, Jorn Vernee wrote:
>>> Hi,
>>>
>>> Please review the following patch that fixes handling of long/double
>>> returns for the tryFinally MethodHandle combinator.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233920
>>> Webrev: http://cr.openjdk.java.net/~jvernee/8233920/webrev.01/
>>> (Testing=tier1)
>>>
>>> FWIW, I also added some missing close tags to the javadoc in the
>>> InvokerBytecodeGenerator class (IntelliJ was warning about this).
>>>
>>> As a heads-up; I'm not a committer on the jdk project, so a sponsor
>>> would be needed to push this.
>>>
>>> Thanks,
>>> Jorn