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

[Vladimir Kozlov]

Generating uncommon trap during parsing when length < 0 works:

@@ -3125,13 +3126,26 @@ Node* GraphKit::new_array(Node* klass_no

    Node* javaoop = set_output_for_allocation(alloc, ary_type, raw_mem_only);

+    Node* ccast = alloc->make_ideal_length(ary_type, &_gvn);
+    if (ccast != length && length_type->_hi < 0) {
+      uncommon_trap(Deoptimization::Reason_unreached,
+                    Deoptimization::Action_reinterpret,
+                    NULL, "negative length", true);
+    }
+
    // Cast length on remaining path to be as narrow as possible
    if (map()->find_edge(length) >= 0) {
      Node* ccast = alloc->make_ideal_length(ary_type, &_gvn);

Vladimir

Tom Rodriguez wrote:
> 
> On Apr 1, 2009, at 2:50 PM, Vladimir Kozlov wrote:
> 
>> You are absolutely right.
>> The problem is the current code replace data path with TOP
>> after the full igvn but control path stays alive.
>> My changes remove CastII if it can produce TOP so the data path
>> stays alive also.
>>
>> I like your suggestion to cut the control path during parsing but
>> it is against our rule which is to let IGVN do cutting
>> since during parsing graph is not complete.
> 
> I wasn't suggesting that we do this during parse.  It should be part of 
> AllocateArrayNode::Ideal when can_reshape is true.
> 
> tom
> 
>>
>>
>> Vladimir
>>
>> Tom Rodriguez wrote:
>>> Isn't this just delaying the problem?  If you hide the negative value 
>>> behind something that won't get simplified until the full igvn pass 
>>> then you're back where you started, aren't you?  Or is this something 
>>> that would only be a problem during parse?
>>
>>
>>
>>> The root of the problem is that the negative length effectively 
>>> proves that the control flow following the allocation is all dead 
>>> since it will throw an exception but our graph doesn't express this.  
>>> The allocation stays around and simply becomes a slow path call that 
>>> will throw the exception but any control flow that follows it is 
>>> actually dead but can't fold up.  Maybe we need to hammer the fall 
>>> through projection of the AllocateArrayNode to be a HaltNode to 
>>> indicate that the fall through path is unreachable?
>>> tom
>>> On Apr 1, 2009, at 11:57 AM, Vladimir Kozlov wrote:
>>>> I added test case suggested by John.
>>>> I also replaced the code in GraphKit::new_array
>>>> with the _gvn.transform(ccast) call and the result's
>>>> type check as John suggested. I think it is safe
>>>> to not delay the transform() call since the TOP result
>>>> will not be used.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~kvn/6823453/webrev.01
>>>>
>>>> Fixed 6823453: DeoptimizeALot causes fastdebug server jvm to fail 
>>>> with assert(false,"unscheduable graph")
>>>>
>>>> Problem:
>>>> The code in GraphKit::new_array replaces a negative value with
>>>> TOP (CastII(-1)#0) for all uses if it is used as new array length.
>>>> Incorrect ideal graph is generated as result.
>>>>
>>>> Solution:
>>>> Don't replace a value with TOP.
>>>> Added regression test.
>>>>
>>>> Reviewed by:
>>>>
>>>> Fix verified (y/n): y, bug tests
>>>>
>>>> Other testing:
>>>> JPRT
>>>>
>