[jdk17] RFR: 8268362: [REDO] C2 crash when compile negative Arrays.copyOf length after loop [v2]

[Vladimir Kozlov]

On Thu, 17 Jun 2021 13:28:07 GMT, Hui Shi <hshi at openjdk.org> wrote:

>> src/hotspot/share/opto/graphKit.cpp line 1192:
>> 
>>> 1190:       _gvn.set_type_bottom(ccast);
>>> 1191:       record_for_igvn(ccast);
>>> 1192:       alen = ccast;
>> 
>> I think we need GraphKit wrapper method which execute this code to avoid duplication here, in `arraycopy_move_allocation_here()` and in `new_array()`.
>> Add comment why we can't do `transform()`.
>
> @vnkozlov  Thanks for your review!
> 
> GraphKit::load_array_length code is different with new_array() and arraycopy_move_allocation_here(), it doesn't replace_in_map. Currently GraphKit::cast_replace_array_length_post_allocation warpper is used in new_array() and arraycopy_move_allocation_here().
> 
> Would you prefer a wrapper for all these three places? It would be like following code.
> 
> Node* GraphKit::cast_array_length_post_allocation(AllocateArrayNode* alloc, const TypeOopPtr* oop_type, bool replace_map /false in load_array_length/) {
>   Node* length = alloc->in(AllocateNode::ALength);
>   if (replace_map == false || map()->find_edge(length) >= 0) {
>     Node* ccast = alloc->make_ideal_length(oop_type, &_gvn);
>     if (ccast != length) {
>       _gvn.set_type_bottom(ccast);
>       record_for_igvn(ccast);
>       if (replace_map) {
>         replace_in_map(length, ccast);
>       }
>       return cast;
>     }
>   }
>   return length;
> }

All places, please. I prefer different names for this method and its parameters `GraphKit::array_ideal_length(alloc , ary_type, replace_length_in_map)`. No need for parameter's comment in declaration - it is clear from name.

-------------

PR: https://git.openjdk.java.net/jdk17/pull/49