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


Reply via email to