John, Thanks for review!
Updated webrev: http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.02/ See my comments inline. Also, integrated some pending cleanups (e.g. improved selectAlternative detection). Best regards, Vladimir Ivanov On 2/24/14 9:46 PM, John Rose wrote: > On Feb 20, 2014, at 9:57 AM, Vladimir Ivanov > <vladimir.x.iva...@oracle.com <mailto:vladimir.x.iva...@oracle.com>> wrote: > >> Updated webrev: >> http://cr.openjdk.java.net/~vlivanov/8027827/final/webrev.01/ >> >> I finally figured out how to make caching work. This webrev contains >> these changes. >> >> I changed LF representation a bit and added 2 auxiliary method handles >> - argument boxing and wrapping into Object[] and result unboxing. >> These operations depend on actual type and can't be shared among >> arbitrary combinators with the same basic type. They are used only >> during LF interpretation and are completely ignored in compiled LFs. > > This is a good step forward, thanks! > > Some comments: > > I prefer the bounds check expression pos+3 < lambdaForm.names.length. > (One integer constant, limit to right.) Fixed. > The predicate isGuardWithCatch must test all three subforms. Or else > there must be assertions to ensure that names[pos+2] is of the expected > form. The problem is that LF's can sometimes be edited (e.g., by > binding operations) and there is no insurance that your pattern of three > expressions will be preserved in all cases. Fixed. > I see you are trying to do unboxing elimination here; this is not a safe > or effective way to do it, IMO. Put in a "FIXME" comment and file a bug > to deal better with unboxing ops in LFs. I have some WIP code toward > this end which we can talk about. You've probably seen of that > business, about internally LF marking expressions as intrinsics to guide > bytecode generation. Added a comment. Will file a bug once this change is in. > Why is the logic about cachedLambdaForm commented out? It looks > correct, but is there a bug? I think you looked at a stale version. Latest webrev have caching enabled. I had some problems with sharing before because boxing/unboxing is specific for different types and couldn't be shared among combinators with the same basic type. > Consider replacing GUARD_WITH_CATCH with Lazy.NF_guardWithCatch, and > using the NF instead of MH for the intrinsic. Done. Also updated other similar places in MethodHandleInfo. > > — John _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
