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

[Vladimir Kozlov]

Thanks. I fixed typo.

Vladimir

Tom Rodriguez wrote:
> 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
>>>>>
>>>>
>