[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Request for reviews (XL): 6894206: JVM needs a way to traverse method	handle structures
From:       Vladimir.Kozlov () Sun ! COM (Vladimir Kozlov)
Date:       2009-11-30 20:35:49
Message-ID: 4B142CA5.20104 () sun ! com
[Download RAW message or body]

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).

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.

And you miss
#undef VM_INTRINSIC_INFO


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).

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

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

  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;
  111     }

  115       if (cur_slot < arg_slot) {
                         ^ <=
  116         if (cur_slot == arg_slot)
  117           arg_type = bt;
  118         break;
  119       }

MethodHandleCompiler::make_invoke()
567   Unimplemented(); <<<< ????????????????

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

Vladimir

Christian Thalinger wrote:
> Here is the first part of the method handle walker:
> 
> http://cr.openjdk.java.net/~twisti/6894206/webrev.01/
> 
> The bytecode adapter generation part for the compilers is another
> webrev, which will be posted later.
> 
> -- Christian
> 

[prev in list] [next in list] [prev in thread] [next in thread]