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

[John Rose]

On Apr 1, 2009, at 3:00 PM, 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.

For the record, if a data value can top out during parsing it can top  
out later also.  For example, a phi can collapse at any time due to  
upstream control flow changes, revealing a constant value.  That's why  
we have the rule to let IGVN do the cutting.

Therefore, special-case bug fixes that remove bad patterns during  
parsing sometimes hide corresponding bugs in later phases.

The parser is is opportunistically inserting a CastII to narrow  
information about length downstream.  If that CastII ever tops out  
later on, it also has to be true that the control edges controlling  
the affected calculations have to top out also.

The graph handles some paradoxes well, primarily topped-out control,  
but not data-only tops.  I think the design is *not* to try a graph  
repair on such paradoxes, such as aggressively killing control edges,  
because data-tops often signal bugs.  IIRC, this annoying invariant is  
there as a heuristic to find bugs.  But it causes bugs like this one.

I guess this means (as Tom suggested) that the optimization of  
outgoing control edge of an AllocateArray has to be at least as smart  
as the CastII node, or else when we get to scheduling we'll have a top  
data for a non-top control.  Currently, it is only as smart as a  
CallNode, which naturally doesn't know about the control effects of  
the AA's length input.  There are extra smarts during macro expansion  
due to the exposure of the initial_slow_test initially created in  
GK::new_array, but they might not be smart enough to top out the  
fallthrough edge.

-- John