Request for reviews (XS): 6823453: DeoptimizeALot causes fastdebug server jvm to fail with assert(false,"unscheduable graph")

[Vladimir Kozlov]

Thank you, John,

You suggestion will only works if we execute _gvn.transform(ccast)
but we delay this call. And the question is why we delay it here?

There is the comment in narrow_size_type() explaining why we use 0
so I would still keep it this way:

   // Negative length arrays will produce weird intermediate dead fast-path code


Vladimir

John Rose wrote:
> On Mar 31, 2009, at 10:24 PM, Vladimir Kozlov wrote:
> 
>> The code in GraphKit::new_array replaces a negative constant with
>> TOP (CastII(-1)#0) for all uses if it is used as new array length.
>> Incorrect ideal graph is generated as result.
> 
> I think there the bug may still be present if length is not constant but 
> in a negative range.
> 
> Change new byte[(byte)1.0E10] to new byte[(byte)var_1 - 128] and see 
> what happens.
> 
> In that case, the type of length is int:[-256..-1], and your change will 
> not have an effect.
> 
> I think this might be a more robust version of your change:
> 
>    if (map()->find_edge(length) >= 0) {
>      Node* ccast = alloc->make_ideal_length(ary_type, &_gvn);
>      if (ccast != length) {
>        _gvn.set_type_bottom(ccast);
>  +     if (!_gvn.type(ccast)->empty()) {
>      record_for_igvn(ccast);
>      replace_in_map(length, ccast);
>  +     }
>      }
> 
> (BTW, the logic of narrow_size_type is a little funky, since it returns 
> 0 for top.  That was fixing some other, similar bug, I think.  But maybe 
> it should have returned a sentinel value of -1 instead of 0.  That would 
> not be a point-fix.)
> 
> -- John