for review (XXL): 6655638 method handles for invokedynamic

[Christian Thalinger]

On Tue, 2009-03-10 at 15:48 -0700, John Rose wrote:
> Thanks for the reviews so far.  I've posted the remainder of  
> meth.patch here:
> 
> 6655638: dynamic languages need method handles
>    http://cr.openjdk.java.net/~jrose/6655638/webrev.00/  (not yet  
> integrated)

As always, I try to do a review.

---

src/cpu/x86/vm/templateInterpreter_x86_32.cpp:

-  const int method_stack = (method->max_locals() + method->max_stack()) *
+  const int extra_stack = methodOopDesc::extra_stack();
+  const int method_stack = (method->max_locals() + method->max_stack() + extra_stack) *
                            Interpreter::stackElementWords();
-  return overhead_size + method_stack + stub_code;
+  return overhead_size + method_stack + extra_stack + stub_code;

Adding extra_stack a second time on return seems wrong.

---

src/share/vm/runtime/globals.hpp:

+  product(bool, MethodHandleSupport, false,                                 \
+          "support method handles (true by default under JSR 292")          \

Closing ) is missing in the description.

---

src/share/vm/prims/methodHandles.cpp:

391   assert(methodOopDesc::invalid_vtable_index - 10 > VM_INDEX_UNINITIALIZED, "Java sentinel value");

At least to me it's not obvious why -10.

539 void MethodHandles::expand_MemberName(Handle mname, int suppress, TRAPS) {

It seems suppress uses some hardcoded values.  It would be better to use
some defines or enum values.

---

src/cpu/x86/vm/methodHandles_x86.cpp:

  78   // rbx: methodOop
  79   // rcx: receiver method handle (must load from sp[MethodTypeForm.vmslots])
  80   // rsi: sender SP (must preserve)
  81   // rdx: garbage temp, blown away
  82 
  83   Register rbx_method = rbx;
  84   Register rcx_recv   = rcx;
  85   Register rax_mtype  = rax;
  86   Register rdx_temp   = rdx;

Seems the comments do not match the code, because rsi is not used in the
code.

---

One minor thing is that the copyright year is not updated in some (or
all?) files.

-- Christian