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.ver...@oracle.com> > À: "Claes Redestad" <claes.redes...@oracle.com>, "core-libs-dev" > <core-libs-dev@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
