Yes, please file this bug. I'm working on MH-related optimizations, and I am finding that preserving the required code pattern is blocking some optimizations, notably making customized variations of direct MHs.
(The required code pattern is that the DMH in Local#0 has a member field which supplies the missing argument to the outgoing linker call. Making a variant DMH with a customized calling sequence is possible, but then it has wrong interactions with the MethodHandleInfo API. It would be easier to make a customized non-DMH, with no member field.) — John On Aug 20, 2013, at 12:16 PM, serguei.spit...@oracle.com wrote: > John, > > Thank you for the comments! > In fact, I was very reluctant to implement it the way as it is in the webrev. > I'm in favor of the choice #3, and think, it is much better from the > stability point of view. > Restoring the MemberName slot at deoptimization is not going to cause a > performance degradation. > > If you and Christian are Ok with it I can file a new compiler bug to cover > this issue. > > Thanks, > Serguei > > > On 8/12/13 3:30 PM, John Rose wrote: >> This fix will be delicate and may have regressions if the exact code shape >> (of the PopFrame-ed invokestatic call) changes. >> >> Note that member_name_arg_or_null assumes that the value in Local#0 is a >> DMH; there will be asserts thrown if this fails. It also assumes that the >> member name held by the DMH in fact corresponds to the linker call >> (linkToStatic, linkToVirtual, etc.) being PopFrame-ed. >> >> In fact, the linker call might in some cases be run from another source than >> "aload0; getfield member". Requiring that this correspondence exist always >> is a new internal interface that is hard to document and verify; it may also >> inhibit present or future optimizations. >> >> There are other approaches that might be more robust: >> 1. Do not allow PopFrame to linker calls, since they (by definition) throw >> away their trailing MemberName argument. >> 2. Do not make such frames visible to the user. >> 3. Modify the linker primitives to store a copy of their trailing MemberName >> argument to a new slot in the interpreter frame and compiled frame. Be sure >> to populate this new slot on deoptimization. >> >> — John >> >> P.S. Sorry about the delay in commenting; I am just digging out from under >> JVMLS business! >> >> On Jul 30, 2013, at 10:00 AM, serguei.spit...@oracle.com wrote: >> >>> The updated webrev: >>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.3/ >>> >>> Thanks, >>> Serguei >>> >>> On 7/29/13 10:11 PM, David Holmes wrote: >>>> Hi Serguei, >>>> >>>> I'm fine with restoring to what was in the first webrev. >>>> >>>> Further trimming of the JVMTI code is something the embedded folk could >>>> look at. It may not be worthwhile. >>>> >>>> Thanks, >>>> David >>>> >>>> On 30/07/2013 3:05 PM, serguei.spit...@oracle.com wrote: >>>>> On 7/29/13 8:22 PM, David Holmes wrote: >>>>>> On 30/07/2013 10:34 AM, serguei.spit...@oracle.com wrote: >>>>>>> Christian and David, >>>>>>> >>>>>>> Thank you for the quick review and comments! >>>>>>> >>>>>>> This is a new version of webrev: >>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.2 >>>>>>> >>>>>> Sorry. You need that guard after all - at least you do if you continue >>>>>> to use it in interpreterRuntime - otherwise member_name_arg_or_null >>>>>> will not exist: >>>>>> >>>>>> __ call_VM(rax, CAST_FROM_FN_PTR(address, >>>>>> InterpreterRuntime::member_name_arg_or_null), rax, rdx, rsi); >>>>>> >>>>> You are right, nice catch again. >>>>> Probably, it was the reason, I did not remove the #if/#endif in the >>>>> first place. :) >>>>> >>>>>> I'm a little surprised that the assembly code does not have that whole >>>>>> section guarded with INCLUDE_JVMTI - perhaps it should? >>>>> It should. >>>>> But it can be non-trivial because of other dependencies like the above. >>>>> To make it right, both _remove_activation_preserving_args_entry and >>>>> generate_earlyret_entry_for >>>>> must be isolated with #if INCLUDE_JVMTI. >>>>> Then more files have to be involved in this chain of changes: >>>>> interpreter/templateInterpreter.hpp >>>>> interpreter/templateInterpreter.hpp >>>>> memory/universe.hpp >>>>> memory/universe.cpp >>>>> code/codeCache.hpp >>>>> code/codeCache.cpp >>>>> . . . etc .. >>>>> >>>>> Please, note, that the HOTSWAP macro is used in many places as well. >>>>> I'm not sure we still need it, so that another decision is needed for it. >>>>> >>>>> So, it seems that this kind of clean up is going far beyond my fix. >>>>> My suggestion is to restore the "#if INCLUDE_JVMTI" in 3 >>>>> platform-dependent files as it was in the webrev v1. >>>>> I'm a little bit reluctant to open another clean-up bug for this issue >>>>> but maybe it is needed. >>>>> Please, let me know if you are comfortable with this solution. >>>>> >>>>> Thanks, >>>>> Serguei >>>>> >>>>>> Run this through a JPRT test build for productEmb to check that the >>>>>> minimal VM builds ok. >>>>>> >>>>>> David >>>>>> ----- >>>>>> >>>>>>> Thanks, >>>>>>> Serguei >>>>>>> >>>>>>> >>>>>>> On 7/28/13 9:11 PM, David Holmes wrote: >>>>>>>> Hi Serguei, >>>>>>>> >>>>>>>> On 26/07/2013 10:14 AM, serguei.spit...@oracle.com wrote: >>>>>>>>> Please, review the fix for: >>>>>>>>> bug: http://bugs.sun.com/view_bug.do?bug_id=7187554 >>>>>>>>> jbs: https://jbs.oracle.com/bugs/browse/JDK-7187554 >>>>>>>>> >>>>>>>>> Open webrev: >>>>>>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/7187554-JVMTI-JSR292.1 >>>>>>>>> >>>>>>>>> >>>>>>>> In the templateInterpreter code why did you put this guard on your new >>>>>>>> code (from x86_32 version): >>>>>>>> >>>>>>>> 1923 #if INCLUDE_JVMTI >>>>>>>> >>>>>>>> when the whole chunk of code this is situated in is specifically for >>>>>>>> JVMTI support >>>>>>>> >>>>>>>> 1824 // >>>>>>>> 1825 // JVMTI PopFrame support >>>>>>>> 1826 // >>>>>>>> >>>>>>>> ??? >>>>>>>> >>>>>>>> David >>>>>>>> ----- >>>>>>>> >>>>>>>>> Summary: >>>>>>>>> Restore the appendix argument of a polymorphic intrinsic call >>>>>>>>> needed for a invokestatic re-execution after JVMTI PopFrame(). >>>>>>>>> >>>>>>>>> Description >>>>>>>>> When JVMTI's PopFrame removes a frame that was called via a call >>>>>>>>> site >>>>>>>>> that >>>>>>>>> takes an appendix and that call site is reexecuted the appendix is >>>>>>>>> not on >>>>>>>>> the stack anymore because it got removed by the adapter. >>>>>>>>> This fix is to detect such a case and push the appendix on the >>>>>>>>> stack >>>>>>>>> again before reexecution. >>>>>>>>> >>>>>>>>> >>>>>>>>> Testing: >>>>>>>>> UTE tests - in progress: vm.mlvm.testlist, nsk.jvmti.testlist, >>>>>>>>> nsk.jdi.testlist >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Serguei >
