Bug ID: JDK-6603492 multianewarray should generate same code as anewarray for 1-d arrays

Type: Enhancement
Component: hotspot
Sub-Component: compiler
Affected Version: 6u6,7

Priority: P3
Status: Closed
Resolution: Fixed
OS: generic
CPU: generic

Submitted: 2007-09-11
Updated: 2011-03-08
Resolved: 2011-03-08

JDK 6	JDK 7	Other
6u10Fixed	7Fixed	hs11Fixed

Here's a missing bit of 6337834, which reworked new, newarray, and anewarray.
The multianewarray instruction should use the new graph whenever possible.
At least, it should use it when the dimension of the array is 1.
Perhaps other common cases should be supported also.

SUGGESTED FIX The fix integrated is in http://javaweb.sfbay/~jrose/webrev/6603492/ . Here is the diff: #!/bin/sh patch=patch ws=${1:-`pwd`} cd $ws/src/share/vm/opto sccs edit parse3.cpp $patch parse3.cpp <<'EOF.src/share/vm/opto/parse3.cpp' *** /export/jrose/ws/bugs0907/webrevs/6603492/src/share/vm/opto/parse3.cpp- Wed Sep 19 15:57:27 2007 --- parse3.cpp Wed Sep 19 15:52:35 2007 *************** *** 351,360 **** --- 351,383 ---- Node* obj = new_array(makecon(array_klass), count_val); // Push resultant oop onto stack push(obj); } + // Expand simple expressions like new int[3][5] and new Object[2][nonConLen]. + // Also handle the degenerate 1-dimensional case of anewarray. + Node* Parse::expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions) { + Node* length = lengths[0]; + assert(length != NULL, ""); + Node* array = new_array(makecon(TypeKlassPtr::make(array_klass)), length); + if (ndimensions > 1) { + jint length_con = find_int_con(length, -1); + guarantee(length_con >= 0, "non-constant multianewarray"); + ciArrayKlass* array_klass_1 = array_klass->as_obj_array_klass()->element_klass()->as_array_klass(); + const TypePtr* adr_type = TypeAryPtr::OOPS; + const Type* elemtype = _gvn.type(array)->is_aryptr()->elem(); + uint shift = LogBytesPerWord; + uint header = arrayOopDesc::base_offset_in_bytes(T_OBJECT); + for (jint i = 0; i < length_con; i++) { + Node* elem = expand_multianewarray(array_klass_1, &lengths[1], ndimensions-1); + intptr_t offset = header + ((intptr_t)i << shift); + Node* eaddr = basic_plus_adr(array, offset); + store_oop_to_array(control(), array, eaddr, adr_type, elem, elemtype, T_OBJECT); + } + } + return array; + } void Parse::do_multianewarray() { int ndimensions = iter().get_dimensions(); // the m-dimensional array *************** *** 362,455 **** ciArrayKlass* array_klass = iter().get_klass(will_link)->as_array_klass(); assert(will_link, "multianewarray: typeflow responsibility"); // Note: Array classes are always initialized; no is_initialized check. ! if (ndimensions > 5) { uncommon_trap(Deoptimization::Reason_unhandled, Deoptimization::Action_none); return; } kill_dead_locals(); - // Can use _multianewarray instead of _anewarray or _newarray - // if only one dimension - if( ndimensions == 1 && array_klass->is_type_array_klass() ) { - // If this is for a basic type, call code for do_newarray instead - BasicType element_type = array_klass->as_type_array_klass()->element_type(); - do_newarray(element_type); - return; - } - - ciObjArrayKlass* obj_array_klass = array_klass->as_obj_array_klass(); - - // find the element type (etype) - ciKlass* element_klass = obj_array_klass->base_element_klass(); - // Base_element is either an instance-klass or a type-array but NOT - // a basic type. We really wanted the klass of a basic type; since that's - // not available we have to test for type-array here. - const Type* element_type = element_klass->is_type_array_klass() - ? Type::get_const_basic_type(element_klass->as_type_array_klass()->element_type()) - : TypeInstPtr::make(TypePtr::BotPTR, element_klass->as_instance_klass()); - - int mdimensions = obj_array_klass->dimension(); - // get the lengths from the stack (first dimension is on top) ! Node** length = NEW_RESOURCE_ARRAY(Node*, ndimensions + 1); length[ndimensions] = NULL; // terminating null for make_runtime_call ! for (int j = ndimensions-1; j >= 0 ; j--) length[j] = pop(); ! // construct the array type ! const Type* prev_type = element_type; ! ciKlass* prev_array = element_klass->is_type_array_klass() ? element_klass : NULL; ! ! // fill the lowest dimensions with unknown sizes ! for (int index = 0; index < mdimensions - ndimensions; index++) { ! const TypeAry* arr0 = TypeAry::make(prev_type, TypeInt::POS); ! prev_type = TypeAryPtr::make(TypePtr::BotPTR, arr0, prev_array, true, 0); ! prev_array = NULL; // array klasses can be lazy, except the first } ! // Fill in the dimensions with known sizes (passed in the JVM stack) ! for (int i = 0; i < ndimensions; i++) { ! const Type* count_type = TypeInt::POS; ! TypePtr::PTR ptr = TypePtr::BotPTR; ! // For the outermost dimension, try to get a better type than POS for the ! // size. We don't do this for inner dimmensions because we lack the ! // support to invalidate the refined type when the base array is modified ! // by an aastore, or when it aliased via certain uses of an aaload. ! if (i == ndimensions - 1) { ! const Type* count_range_type = length[0]->bottom_type()->join(count_type); ! // Only improve the type if the array length is non-negative. ! if (!count_range_type->empty()) { ! count_type = count_range_type; ! ptr = TypePtr::NotNull; ! } ! } ! assert(count_type->is_int(), "must be integer"); ! const TypeAry* arr0 = TypeAry::make(prev_type, (TypeInt*)count_type); ! prev_type = TypeAryPtr::make(ptr, arr0, prev_array, true, 0); ! prev_array = NULL; // array klasses can be lazy, except the first } - const TypeAryPtr* arr = (const TypeAryPtr*)prev_type; address fun = NULL; switch (ndimensions) { ! case 1: fun = OptoRuntime::multianewarray1_Java(); break; ! case 2: fun = OptoRuntime::multianewarray2_Java(); break; ! case 3: fun = OptoRuntime::multianewarray3_Java(); break; ! case 4: fun = OptoRuntime::multianewarray4_Java(); break; ! case 5: fun = OptoRuntime::multianewarray5_Java(); break; ! default: ShouldNotReachHere(); }; Node* c = make_runtime_call(RC_NO_LEAF | RC_NO_IO, OptoRuntime::multianewarray_Type(ndimensions), fun, NULL, TypeRawPtr::BOTTOM, makecon(TypeKlassPtr::make(array_klass)), length[0], length[1], length[2], length[3], length[4]); Node* res = _gvn.transform(new (C, 1) ProjNode(c, TypeFunc::Parms)); ! Node *cast = _gvn.transform( new (C, 2) CheckCastPPNode(control(), res, arr) ); ! push( cast ); } --- 385,467 ---- ciArrayKlass* array_klass = iter().get_klass(will_link)->as_array_klass(); assert(will_link, "multianewarray: typeflow responsibility"); // Note: Array classes are always initialized; no is_initialized check. ! enum { MAX_DIMENSION = 5 }; ! if (ndimensions > MAX_DIMENSION || ndimensions <= 0) { uncommon_trap(Deoptimization::Reason_unhandled, Deoptimization::Action_none); return; } kill_dead_locals(); // get the lengths from the stack (first dimension is on top) ! Node* length[MAX_DIMENSION+1]; length[ndimensions] = NULL; // terminating null for make_runtime_call ! int j; ! for (j = ndimensions-1; j >= 0 ; j--) length[j] = pop(); ! // The original expression was of this form: new T[length0][length1]... ! // It is often the case that the lengths are small (except the last). ! // If that happens, use the fast 1-d creator a constant number of times. ! const jint expand_limit = MIN2((juint)MultiArrayExpandLimit, (juint)100); ! jint expand_count = 1; // count of allocations in the expansion ! jint expand_fanout = 1; // running total fanout ! for (j = 0; j < ndimensions-1; j++) { ! jint dim_con = find_int_con(length[j], -1); ! expand_fanout *= dim_con; ! expand_count += expand_fanout; // count the level-J sub-arrays ! if (dim_con < 0 ! || dim_con > expand_limit ! || expand_count > expand_limit) { ! expand_count = 0; ! break; ! } } ! // Can use multianewarray instead of [a]newarray if only one dimension, ! // or if all non-final dimensions are small constants. ! if (expand_count > 0 && expand_count <= expand_limit) { ! Node* obj = expand_multianewarray(array_klass, &length[0], ndimensions); ! push(obj); ! return; } address fun = NULL; switch (ndimensions) { ! //case 1: fun = OptoRuntime::multianewarray1_Java(); break; ! case 2: fun = OptoRuntime::multianewarray2_Java(); break; ! case 3: fun = OptoRuntime::multianewarray3_Java(); break; ! case 4: fun = OptoRuntime::multianewarray4_Java(); break; ! case 5: fun = OptoRuntime::multianewarray5_Java(); break; ! default: ShouldNotReachHere(); }; Node* c = make_runtime_call(RC_NO_LEAF | RC_NO_IO, OptoRuntime::multianewarray_Type(ndimensions), fun, NULL, TypeRawPtr::BOTTOM, makecon(TypeKlassPtr::make(array_klass)), length[0], length[1], length[2], length[3], length[4]); Node* res = _gvn.transform(new (C, 1) ProjNode(c, TypeFunc::Parms)); ! ! const Type* type = TypeOopPtr::make_from_klass_raw(array_klass); ! ! // Improve the type: We know it's not null, exact, and of a given length. ! type = type->is_ptr()->cast_to_ptr_type(TypePtr::NotNull); ! type = type->is_aryptr()->cast_to_exactness(true); ! ! const TypeInt* ltype = _gvn.find_int_type(length[0]); ! if (ltype != NULL) ! type = type->is_aryptr()->cast_to_size(ltype); ! ! // We cannot sharpen the nested sub-arrays, since the top level is mutable. ! ! Node* cast = _gvn.transform( new (C, 2) CheckCastPPNode(control(), res, type) ); ! push(cast); ! ! // Possible improvements: ! // - Make a fast path for small multi-arrays. (W/ implicit init. loops.) ! // - Issue CastII against length[*] values, to TypeInt::POS. } EOF.src/share/vm/opto/parse3.cpp cd $ws/src/share/vm/opto sccs edit parse.hpp $patch parse.hpp <<'EOF.src/share/vm/opto/parse.hpp' *** /export/jrose/ws/bugs0907/webrevs/6603492/src/share/vm/opto/parse.hpp- Wed Sep 19 15:57:27 2007 --- parse.hpp Wed Sep 19 11:42:08 2007 *************** *** 469,478 **** --- 469,479 ---- // implementation of object creation bytecodes void do_new(); void do_newarray(BasicType elemtype); void do_anewarray(); void do_multianewarray(); + Node* expand_multianewarray(ciArrayKlass* array_klass, Node* *lengths, int ndimensions); // implementation of jsr/ret void do_jsr(); void do_ret(); EOF.src/share/vm/opto/parse.hpp cd $ws/src/share/vm/opto sccs edit c2_globals.hpp $patch c2_globals.hpp <<'EOF.src/share/vm/opto/c2_globals.hpp' *** /export/jrose/ws/bugs0907/webrevs/6603492/src/share/vm/opto/c2_globals.hpp- Wed Sep 19 15:57:28 2007 --- c2_globals.hpp Wed Sep 19 15:57:21 2007 *************** *** 148,157 **** --- 148,161 ---- "Don't use profile_trip_cnt() to restrict unrolling until " \ "unrolling would push the number of unrolled iterations above " \ "UnrollLimitForProfileCheck. A higher value allows more " \ "unrolling. Zero acts as a very large value." ) \ \ + product(intx, MultiArrayExpandLimit, 6, \ + "Maximum number of individual allocations in an inline-expanded " \ + "multianewarray instruction") \ + \ notproduct(bool, TraceProfileTripCount, false, \ "Trace profile loop trip count information") \ \ develop(bool, OptoCoalesce, true, \ "Use Conservative Copy Coalescing in the Register Allocator") \ EOF.src/share/vm/opto/c2_globals.hpp

16-02-2008

EVALUATION This is a follow-on to 6337834 (remove unneeded zeroing during object allocation). It applies code cleanups done in 6337834 to multianewarray. The main advantage is that it gets rid of some bug-laden code from the previous millennium.

01-10-2007

SUGGESTED FIX At a minimum: ------- src/share/vm/opto/parse3.cpp ------- --- /tmp/sccs.f4aapo Tue Sep 11 14:36:33 2007 +++ src/share/vm/opto/parse3.cpp Sat Sep 8 19:09:18 2007 @@ -364,92 +364,64 @@ // Note: Array classes are always initialized; no is_initialized check. - if (ndimensions > 5) { + enum { MAX_DIMENSION = 5 }; + if (ndimensions > MAX_DIMENSION) { uncommon_trap(Deoptimization::Reason_unhandled, Deoptimization::Action_none); return; } + Node* array_klass_con = makecon(TypeKlassPtr::make(array_klass)); + kill_dead_locals(); - // Can use _multianewarray instead of _anewarray or _newarray - // if only one dimension - if( ndimensions == 1 && array_klass->is_type_array_klass() ) { - // If this is for a basic type, call code for do_newarray instead - BasicType element_type = array_klass->as_type_array_klass()->element_type(); - do_newarray(element_type); - return; - } - - ciObjArrayKlass* obj_array_klass = array_klass->as_obj_array_klass(); - - // find the element type (etype) - ciKlass* element_klass = obj_array_klass->base_element_klass(); - // Base_element is either an instance-klass or a type-array but NOT - // a basic type. We really wanted the klass of a basic type; since that's - // not available we have to test for type-array here. - const Type* element_type = element_klass->is_type_array_klass() - ? Type::get_const_basic_type(element_klass->as_type_array_klass()->element_type()) - : TypeInstPtr::make(TypePtr::BotPTR, element_klass->as_instance_klass()); - - int mdimensions = obj_array_klass->dimension(); - // get the lengths from the stack (first dimension is on top) - Node** length = NEW_RESOURCE_ARRAY(Node*, ndimensions + 1); + Node* length[MAX_DIMENSION+1]; length[ndimensions] = NULL; // terminating null for make_runtime_call for (int j = ndimensions-1; j >= 0 ; j--) length[j] = pop(); - // construct the array type - const Type* prev_type = element_type; - ciKlass* prev_array = element_klass->is_type_array_klass() ? element_klass : NULL; + // The original expression was of this form: new T[length0][length1]... - // fill the lowest dimensions with unknown sizes - for (int index = 0; index < mdimensions - ndimensions; index++) { - const TypeAry* arr0 = TypeAry::make(prev_type, TypeInt::POS); - prev_type = TypeAryPtr::make(TypePtr::BotPTR, arr0, prev_array, true, 0); - prev_array = NULL; // array klasses can be lazy, except the first - } - - // Fill in the dimensions with known sizes (passed in the JVM stack) - for (int i = 0; i < ndimensions; i++) { - const Type* count_type = TypeInt::POS; - TypePtr::PTR ptr = TypePtr::BotPTR; - // For the outermost dimension, try to get a better type than POS for the - // size. We don't do this for inner dimmensions because we lack the - // support to invalidate the refined type when the base array is modified - // by an aastore, or when it aliased via certain uses of an aaload. - if (i == ndimensions - 1) { - const Type* count_range_type = length[0]->bottom_type()->join(count_type); - // Only improve the type if the array length is non-negative. - if (!count_range_type->empty()) { - count_type = count_range_type; - ptr = TypePtr::NotNull; - } - } - assert(count_type->is_int(), "must be integer"); - const TypeAry* arr0 = TypeAry::make(prev_type, (TypeInt*)count_type); - prev_type = TypeAryPtr::make(ptr, arr0, prev_array, true, 0); - prev_array = NULL; // array klasses can be lazy, except the first - } - const TypeAryPtr* arr = (const TypeAryPtr*)prev_type; - address fun = NULL; switch (ndimensions) { - case 1: fun = OptoRuntime::multianewarray1_Java(); break; - case 2: fun = OptoRuntime::multianewarray2_Java(); break; - case 3: fun = OptoRuntime::multianewarray3_Java(); break; - case 4: fun = OptoRuntime::multianewarray4_Java(); break; - case 5: fun = OptoRuntime::multianewarray5_Java(); break; - default: ShouldNotReachHere(); + case 1: + // Can use multianewarray instead of [a]newarray if only one dimension. + { Node* obj = new_array(array_klass_con, length[0]); + push(obj); + return; + } + //fun = OptoRuntime::multianewarray1_Java(); break; + case 2: fun = OptoRuntime::multianewarray2_Java(); break; + case 3: fun = OptoRuntime::multianewarray3_Java(); break; + case 4: fun = OptoRuntime::multianewarray4_Java(); break; + case 5: fun = OptoRuntime::multianewarray5_Java(); break; + default: ShouldNotReachHere(); }; Node* c = make_runtime_call(RC_NO_LEAF | RC_NO_IO, OptoRuntime::multianewarray_Type(ndimensions), fun, NULL, TypeRawPtr::BOTTOM, - makecon(TypeKlassPtr::make(array_klass)), + array_klass_con, length[0], length[1], length[2], length[3], length[4]); Node* res = _gvn.transform(new (C, 1) ProjNode(c, TypeFunc::Parms)); - Node *cast = _gvn.transform( new (C, 2) CheckCastPPNode(control(), res, arr) ); - push( cast ); + + const Type* type = TypeOopPtr::make_from_klass_raw(array_klass); + + // Improve the type: We know it's not null, exact, and of a given length. + type = type->is_ptr()->cast_to_ptr_type(TypePtr::NotNull); + type = type->is_aryptr()->cast_to_exactness(true); + + const TypeInt* ltype = _gvn.find_int_type(length[0]); + if (ltype != NULL) + type = type->is_aryptr()->cast_to_size(ltype); + + // We only sharpen the outermost dimension, since it is mutable. + + Node* cast = _gvn.transform( new (C, 2) CheckCastPPNode(control(), res, type) ); + push(cast); + + // Possible improvements: + // - Make a fast path for small multi-arrays. (W/ implicit init. loops.) + // - Issue CastII against length[*] values, to TypeInt::POS. } @@end@@

11-09-2007

Relates :	JDK-6663908 - NegativeArraySizeException is not thrown
Relates :	JDK-6910605 - C2: NullPointerException/ClassCaseException is thrown when C2 with DeoptimizeALot is used