Request for reviews (XL): 6894206: JVM needs a way to traverse method handle structures

[Christian Thalinger]

On Mon, 2009-11-30 at 12:35 -0800, Vladimir Kozlov wrote:
> vmSymbols.cpp
> 
> I think you should define and assert check log2_FLAG_LIMIT
> as we do for log2_SID_LIMIT.
> Also log2_SID_LIMIT == 10 and with log2_FLAG_LIMIT == 4 you
> leave only 6 bits (63, 31 without sign) for klass id.
> Will it be enough? You should add assert to check it in
> vmSymbols::initialize().
> 
> Your asserts should use max values, something like this:
> 
> !   assert(((ID4(1021,1022,1023,15) >> shift) & mask) == 1021, "");
> 
> which will fail because of the above (you have to use 31 instead of 1021).

It's John's code and I don't know if 6 bits are enough, but I did the
changes you suggested.

> Also last methods are not for printouts only so you need
> to modify the comment. Or method_for() is called only
> for debug output? Then you should keep #ifndef PRODUCT.

No, it's used later in 6893268.

> And you miss
> #undef VM_INTRINSIC_INFO

Done.

> src/share/vm/utilities/growableArray.hpp
> Remove you change and use Tom's insert_before() method he added
> for 6892658: C2 should optimize some stringbuilder (when he
> push the change into HS17).

I added Tom's insert_before() until it hits the repositories.

> src/share/vm/prims/methodHandleWalk.hpp
> Add assert in make_stack_value() to verify that
> TokenType and BasicType values fit into 4 bits.

I change ArgToken to be a class in 6893268 and there's no need for
asserts then.  I leave this for now.

> src/share/vm/prims/methodHandleWalk.cpp
> In compute_bound_arg_type() missing check:
> if (arg_slot >= m->size_of_parameters()) return T_VOID;

Added.

> 
>   107     if (!m->is_static()) {
>   108       cur_slot -= type2size[T_OBJECT];
>   109       if (cur_slot == arg_slot)
>   110         arg_type = T_OBJECT;
>               ^
>               return T_OBJECT;

Done.

>   111     }
> 
>   115       if (cur_slot < arg_slot) {
>                          ^ <=

That bug is fixed in 6893268, but I will fix it here.

>   116         if (cur_slot == arg_slot)
>   117           arg_type = bt;
>   118         break;
>   119       }
> 
> MethodHandleCompiler::make_invoke()
> 567   Unimplemented(); <<<< ????????????????

This method is implemented in 6893268.

> argument_count_slow() is used only in assert.
> May be add #ifdef ASSERT around it.

Done.

Here is the updated webrev:

http://cr.openjdk.java.net/~twisti/6894206/webrev.02/

-- Christian