RFR 8218142 [lworld] C1 support for array covariance for aaload

[Ioi Lam]

Hi Fred,

Thanks for the information. I've changed the check to

         if (element_klass->is_java_lang_Object() || 
element_klass->is_interface()) {
           // Array covariance:
           //    (ValueType[] <: Object[])
           //    (ValueType[] <: <any interface>[])
           // We will add a runtime check for flat-ness.
           return true;
         }

Thanks

- Ioi


On 1/31/19 7:15 AM, Frederic Parain wrote:
> Ioi,
>
> I haven't completed the review yet, but I have a question about this
> method:
>
>  150 bool Instruction::maybe_flattened_array() const {
>  151   if (ValueArrayFlatten) {
>  152     ciType* type = declared_type();
>  153     if (type != NULL) {
>  154       if (type->is_value_array_klass()) {
>  155         ciValueKlass* element_klass = 
> type->as_value_array_klass()->element_klass()->as_value_klass();
>  156         if (!element_klass->is_loaded() || 
> element_klass->flatten_array()) {
>  157           // For unloaded value arrays, we will add a runtime 
> check for flat-ness.
>  158           return true;
>  159         }
>  160       } else if (type->is_obj_array_klass()) {
>  161         ciKlass* element_klass = 
> type->as_obj_array_klass()->element_klass();
>  162         if (element_klass->is_java_lang_Object()) {
>  163           // Array covariance (ValueType[] <: Object[])
>  164           // We will add a runtime check for flat-ness.
>  165           return true;
>  166         }
>  167       }
>  168     }
>  169   }
>  170
>  171   return false;
>  172 }
>
> I think this method should also return true when the type
> of the element is an interface. Am I missing something?
>
> Fred
>
>
> On 1/31/19 7:52 AM, Ioi Lam wrote:
>> http://cr.openjdk.java.net/~iklam/valhalla/8218142-aaload-covariance.v01/ 
>>
>> https://bugs.openjdk.java.net/browse/JDK-8218142
>>
>> When loading from an array declared as Object[], now the C1 code 
>> checks if the array is flattened. If so, the loading is done via a 
>> runtime call.
>>
>> Here's an example of the generated code
>>
>>        mov    0x8(%rsi),%eax      ; rsi = array
>>        mov    $0x800000000,%r12
>>        add    %r12,%rax           ; rax = array klass
>>        xor    %r12,%r12           ; why is this here??
>>        mov    0xc(%rax),%eax      ; layout_helper
>>        sar    $0x1d,%eax
>>        cmp    $0xfffffffd,%eax    ; check for 
>> Klass::_lh_array_tag_vt_value
>>        je      L_3                ; slow case
>>
>> L_1   movslq %edx,%rdx           ; non-flattened load
>>        mov    0x10(%rsi,%rdx,4),%eax
>>        shl    $0x3,%rax
>> L_2   ....
>>
>> L_3   mov    %rsi,0x8(%rsp)      ; LoadFlattenedArrayStub slow case
>>        mov    %rdx,(%rsp)
>>        callq  0x00007fe6489c8840  ; runtime_call load_flattened_array
>>        jmpq    L_2
>>
>>
>> The code is pretty big and sub-optimal. My plan is to also get 
>> aastore working first, and then optimize.
>>
>> One possibility is to check if the array is actually an Object[] 
>> array. That should help with the erased generic types.
>>
>>        mov    0x8(%rsi),%eax      ; rsi = array
>> +     cmp    $0x00001234, %eax   ; compressed klass for 
>> [Ljava/lang/Object;
>> +     je     L_1                 ; go to non-flattened load
>>        mov    $0x800000000,%r12
>>
>>
>> Thanks
>> - Ioi