On 11/11/19 03:06, serguei.spit...@oracle.com wrote:
Hi Chris,


On 11/8/19 16:55, Chris Plummer wrote:
Hi Alex,

Comments below:

On 11/8/19 4:39 PM, Alex Menkov wrote:


On 11/08/2019 15:22, Alex Menkov wrote:
Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
I don't really see a resolution in the JDK-8215196 comments as to what is actually broken. Are we sure we want to fix this in the test, and not require different behavior by the compiler (and also clarify the spec)?

This is one more case to the topic "Should optimizations be observable for JVMTI agents?"
which was recently discussed on the serviceability-dev mailing list.
The question is about the following statement of the JVMTI PopFrame spec:
 "Note however, that any changes to the arguments, which occurred in the called method, remain;   when execution continues, the first instruction to execute will be the invoke."

One point is we could consider this statement as a possible side affect which has to be preserved by the JIT compiler. So, the optimization to delete such a code which "looks useless" has to be disabled.

Another point is that it is hard to understand why such a side effect can be really useful. Maybe it was specified like this just because it does not make sense to preserve original argument values at the call side. We can consider to relax the JVMTI PopFrame spec by changing it to something like:  "Note however, that the original argument values are not preserved and can be changed by the called method;"

Forgot to list one more option which is:
  Consider it is Okay for compiler to eliminate useless code,
  so the argument values can be reinitialized by the PopFrame.
  Than this problem becomes just a test bug.


Thanks,
Serguei


Let's wait for other opinions.

Thanks,
Serguei


Currently PopFrame is disabled with JVMCI by [1], so for testing I reverted [1] changes.

Just to be clear - I temporary reverted [1] for test runs.

The description for JDK-8218025 says that the intention is to only disable these capabilities for JDK12. Is there a CR to re-enabled them?

thanks,

Chris
--alex


[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex



Reply via email to