Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8002087

JSR 292: max_stack issues in interpreter code after JSR-292 merge

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Duplicate
    • Icon: P4 P4
    • tbd
    • hs25
    • hotspot
    • generic
    • generic

      It seems the merge of the JSR-292 added some issues related to change of the Method::max_stack().
      Please, see the email exchange below:

      On Oct 30, 2012, at 4:25 PM, serguei.spitsyn@oracle.com wrote:

      > Ok, it seems there are some suspicious fragments in the interpreter code.
      > Christian, could you, please, check and comment the fragments below?
      >
      > This is how the Method::max_stack() is defined:
      >
      > src/share/vm/oops/method.hpp:
      >
      > int verifier_max_stack() const { return _max_stack; }
      > int max_stack() const { return _max_stack + extra_stack_entries(); }
      > void set_max_stack(int size) { _max_stack = size; }
      > . . .
      > static int extra_stack_entries() { return EnableInvokeDynamic ? 2 : 0; }
      >
      >
      > The following code fragments are unaware that the method->max_stack() returns _max_stack + extra_stack_entries() :
      >
      > src/cpu/sparc/vm/cppInterpreter_sparc.cpp:
      > src/cpu/x86/vm/cppInterpreter_x86.cpp:
      >
      > static int size_activation_helper(int callee_extra_locals, int max_stack, int monitor_size) {
      > . . .
      > const int extra_stack = 0; //6815692//Method::extra_stack_entries(); ????
      > return (round_to(max_stack +
      > extra_stack +

      Remove extra_stack.

      > . . .
      > }
      > . . .
      > void BytecodeInterpreter::layout_interpreterState(interpreterState to_fill,
      > . . .
      > int extra_stack = 0; //6815692//Method::extra_stack_entries(); ????
      > to_fill->_stack_limit = stack_base - (method->max_stack() + 1 + extra_stack);

      Remove extra_stack (but keep the +1; see comment nearby).

      > . . .
      > }
      >
      >
      > src/cpu/sparc/vm/templateInterpreter_sparc.cpp:
      >
      > static int size_activation_helper(int callee_extra_locals, int max_stack, int monitor_size) {
      > . . .
      > const int max_stack_words = max_stack * Interpreter::stackElementWords;
      > return (round_to((max_stack_words
      > //6815692//+ Method::extra_stack_words() ????

      The comment needs to be removed.

      > . . .
      > }
      >
      > At the size_activation_helper call sites the second parameter is usually passed as method->max_stack().
      >
      >
      > src/cpu/x86/vm/templateInterpreter_x86_32.cpp:
      > src/cpu/x86/vm/templateInterpreter_x86_64.cpp:
      >
      > int AbstractInterpreter::size_top_interpreter_activation(Method* method) {
      > . . .
      > const int extra_stack = Method::extra_stack_entries();
      > const int method_stack = (method->max_locals() + method->max_stack() + extra_stack) *

      Remove extra_stack.

      > Interpreter::stackElementWords;
      > . . .
      > }
      >
      >
      > src/share/vm/interpreter/oopMapCache.cpp:
      >
      > void OopMapForCacheEntry::compute_map(TRAPS) {
      > . . .
      > // First check if it is a method where the stackmap is always empty
      > if (method()->code_size() == 0 || method()->max_locals() + method()->max_stack() == 0) {
      > _entry->set_mask_size(0);
      > } else {
      > . . .
      > }
      >
      > Above, if the invokedynamic is enabled then the method()->max_stack() can not be 0.
      > We need to check it if this fact does not break the fragment.

      That means we are always generating oop maps even if we wouldn't need them. Let me think more about this...

      -- Chris

            sspitsyn Serguei Spitsyn
            sspitsyn Serguei Spitsyn
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: