John,

sharedRuntime_sparc.cpp:
  Why casting to (int)? Also use pointer_delta(code_end, code_start,1):
+  __ set((int)(intptr_t)(code_end - code_start), temp2_reg);

  You bound L_fail label twice, it should be local in range_check(). Use brx() 
instead of br() since you compare pointers. And use cmp_and_brx_short() if 
delayed instruction is nop().

  Use fatal() instead of guarantee:
guarantee(false, err_msg("special_dispatch=%d", special_dispatch));

interpreter_sparc.cpp:
  In generate_method_entry() use fatal() instead of ShouldNotReachHere():
fatal(err_msg("unexpected method kind: %d", kind));


methodHandles_sparc.cpp:
  In MethodHandles::verify_klass() calls restore() should be after BINDs.

  In MethodHandles::jump_from_method_handle() use cmp_and_br_short(temp, 0, )

  Instead of 100 use strlen(name)+50:
+    char* qname = NEW_C_HEAP_ARRAY(char, 100);
+    jio_snprintf(qname, 100,

sharedRuntime_x86_32.cpp:
sharedRuntime_x86_64.cpp:
  The same problem with L_fail label as in sharedRuntime_sparc.cpp.

templateInterpreter_x86_32.cpp:
templateInterpreter_x86_64.cpp:
  Again use use fatal() instead of ShouldNotReachHere() in 
generate_method_entry()

I see in several files next code pattern. Should we call 
throw_IncompatibleClassChangeError() as we do in other places?:

+  if (!EnableInvokeDynamic) {
+    // rewriter does not generate this bytecode
+    __ should_not_reach_here();
+    return;
+  }

c1_FrameMap.cpp:
  Why is ShouldNotReachHere() for mh_invoke in  
FrameMap::java_calling_convention()?

c1_GraphBuilder.cpp:
  add parenthesis:
const bool is_invokedynamic = code == Bytecodes::_invokedynamic;

nmethod.cpp:
  Don't put printing nmethod's addresses under Verbose flag.

linkResolver.cpp:
  Can you replace infinite for(;;) in resolve_invokedynamic() with finite loop 
since the body is executed once or twice.

templateInterpreter.cpp:
  why you need additional {} around the loop?

constantPoolOop.cpp:
  Why not use guarantee() for bad operants?
  Why you need separate scopes in resolve_bootstrap_specifier_at_impl()?

symbol.cpp:
  The loop in index_of_at() should be for(; scan <= limit; scan++) and after 
loop return -1.

bytecodeInfo.cpp:
Don't add spaces into conditions, looks strange.
Remove commented code for inline ForceInline methods.

callGenerator.cpp:
Please, decide which code to use: +#if 1. And I don't think new code is correct.

graphKit.cpp:
Remove commented debug print.
insert_argument() and remove_argument() are not used.


Vladimir

John Rose wrote:
> As some of you have noticed, Chris Thalinger, Michael Haupt, and I have been 
> working on the mlvm patches [1] for JEP-160 [2] for several months.  These 
> changes make method handles more optimizable.  They refactor lots of "magic" 
> out of the JVM and into more manageable Java code.
> 
> To get an idea of how much "magic" is being removed, consider that the change 
> removes 12,000 lines of non-comment code from the JVM, including much 
> assembly code.  It inserts 4900 lines of non-comment code.
> 
> These changes are now stable enough to integrate.  They pass jtreg tests in a 
> number of execution modes and platforms.  They also correctly run various 
> JRuby and Nashorn test programs.  Although there are no performance gains to 
> boast about at present, these changes clear the ground for long-term 
> optimization work.
> 
> Here is the webrev [3], for review and integration into JDK 8 via 
> hotspot-comp/hotspot/.
> 
> Because of the large size of this change set, we request that reviewers would 
> clearly designate which files they are reviewing.  That way we may be able to 
> divide up the work a little more effectively.
> 
> Also, because of the scope of the change, we may respond to some comments by 
> promising to address an issue in a future change set.  If necessary, we will 
> file tracking bugs to make sure nothing gets dropped.  We have been working 
> on this for months, and expect to make many further changes.
> 
> The immediate need to get the changes in is twofold:  First, some bugs 
> (involving symbolic references off the boot class path) require the new 
> Lambda Form intermediate representation, which is "off-BCP-clean".  Second, 
> we need to commit our pervasive changes to the JVM sooner rather than later, 
> so they can be properly integrated with other pervasive changes, such as 
> metadata changes.
> 
> An associated webrev for hotspot-comp/jdk/ will be posted soon; it is already 
> present on mlvm-dev for the curious to examine.  (This change set also 
> deletes a lot of old code.)
> 
> Thanks in advance,
> — John
> 
> [1] 
> http://hg.openjdk.java.net/mlvm/mlvm/hotspot/file/tip/meth-lazy-7023639.patch
> [2] http://openjdk.java.net/jeps/160
> [3] http://cr.openjdk.java.net/~jrose/7023639/webrev.00/
> 
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev

Reply via email to