Request for reviews (S): 6836054: java/util/Arrays/CopyMethods.java fails on solaris-sparc with IllegalArgumentException

[Tom Rodriguez]

Right.  I get what you're saying and I think I follow it now.  Looks  
good apart from the typo: accept -> except.

tom

On May 11, 2009, at 12:00 PM, Vladimir Kozlov wrote:

> Sorry for long output. Here is the generated code which
> loads the allocation's actual type dynamically:
>
> 195    ConL    ===  0  [[ 196  211 ]]  #long:24
> 190    CastPP  ===  182  11  [[ 176  196  196  206  242 ]]  #java/ 
> lang/Class:NotNull:exact *  Oop:java/lang/Class:NotNull:exact * ! 
> jvms: Array::newInstance @ bci:2 CopyMethods::makeArray @ bci:2  
> CopyMethods::fullTests @ bci:72
> 196    AddP    === _  190  190  195  [[ 197 ]]   Oop:java/lang/ 
> Class:NotNull:exact+24 * !jvms: Array::newInstance @ bci:2  
> CopyMethods::makeArray @ bci:2 CopyMethods::fullTests @ bci:72
> 197    LoadKlass       === _  7  196  [[ 198  203 ]]   
> @rawptr:BotPTR, idx=Raw; # *  Klass: * !jvms: Array::newInstance @  
> bci:2 CopyMethods::makeArray @ bci:2 CopyMethods::fullTests @ bci:72
> 203    CastPP  ===  202  197  [[ 211  211  234 ]]  #klass java/lang/ 
> Object: 0x00000000004bcd58 *  Klass:klass java/lang/Object:  
> 0x00000000004bcd58 * !jvms: Array::newInstance @ bci:2  
> CopyMethods::makeArray @ bci:2 CopyMethods::fullTests @ bci:72
> 234    AllocateArray   ===  202  91  92  8  1 ( 232  203  24  24   
> 10  11  68  130  24  1  1  1  1  1  1  24  11  1  1  1  1 ) [[ 235   
> 236  237  244  245  246 ]]  rawptr:NotNull ( int:>=0, java/lang/ 
> Object:NotNull *, bool, int ) Array::newInstance @ bci:2  
> CopyMethods::makeArray @ bci:2 CopyMethods::fullTests @ bci:72 ! 
> jvms: Array::newInstance @ bci:2 CopyMethods::makeArray @ bci:2  
> CopyMethods::fullTests @ bci:72
>
> Vladimir
>
> Vladimir Kozlov wrote:
>> We don't know statically during a method compilation the actual type
>> because it is loaded dynamically for reflaction allocation.
>> Escape Analysis requires to know the type statically to pass it to  
>> debug info:
>>  const Type *t = local->bottom_type();
>>  // Is it a safepoint scalar object node?
>>  if (local->is_SafePointScalarObject()) {
>>    SafePointScalarObjectNode* spobj = local- 
>> >as_SafePointScalarObject();
>>    ObjectValue* sv = Compile::sv_for_node_id(objs, spobj->_idx);
>>    if (sv == NULL) {
>>      ciKlass* cik = t->is_oopptr()->klass();
>>      assert(cik->is_instance_klass() ||
>>             cik->is_array_klass(), "Not supported allocation.");
>>      sv = new ObjectValue(spobj->_idx,
>>                           new ConstantOopWriteValue(cik- 
>> >encoding()));
>> Vladimir
>> Tom Rodriguez wrote:
>>> I have to admit I don't really follow this logic in the code.  How  
>>> is it that we can allocate something but not know it's actual  
>>> type?  Isn't that required to emit the allocation in the first  
>>> place?
>>>
>>> tom
>>>
>>> On May 7, 2009, at 8:29 PM, Vladimir Kozlov wrote:
>>>
>>>>
>>>> http://cr.openjdk.java.net/~kvn/6836054/webrev.00
>>>>
>>>> Fixed 6836054: java/util/Arrays/CopyMethods.java fails on solaris- 
>>>> sparc with IllegalArgumentException
>>>>
>>>> Problem:
>>>> Escape Analysis requires that allocations should have
>>>> exact type (by using CheckCastPP node) for restoring
>>>> objects during deoptimization. But reflection allocation
>>>> set type to j.l.Object assuming that caller will cast
>>>> to exact type. In the failing case there is no cast
>>>> to array type so debug info contains incorrect type
>>>> (j.l.Object) for scalar replaced array allocation.
>>>> It causes the Exception after deoptimization.
>>>>
>>>> Solution:
>>>> Do not mark an allocation as scalar replaceable if
>>>> it does not have exact type.
>>>>
>>>> Reviewed by:
>>>>
>>>> Fix verified (y/n): y, bugs case
>>>>
>>>> Other testing:
>>>> JPRT, CTW
>>>>
>>>