Request for reviews (S): 6991188: C2 Crashes while compiling method

[Y. S. Ramakrishna]

Hi Vladimir --

Just a general $0.02 comment, without my knowing anything about
what is actually being done here.

Can you give an upper bound on the number of iterations
that would be needed in terms of a suitable metric on the
starting graph? A little more commentary on the upper bound
(if a non-trivial one exists), and the choice and suitability of
the default, would be nice.

Also, perhaps use a defined constant instead of the 10? (Would it
be useful to have it be a diagnostic tunable?)

-- ramki

On 11/02/10 16:59, Tom Rodriguez wrote:
> On Nov 2, 2010, at 4:42 PM, Vladimir Kozlov wrote:
> 
>> Thanks, Tom
>>
>> Tom Rodriguez wrote:
>>> Typo:
>>> +     // Possible infinit build_connection_graph loop.
>> Fixed.
>>
>>> Why 10?  How many passes does it normally take? 
>> Normally only 1-2 passes depending on graph complexity. In the bug test - 2, in JBB I saw 3 once.
>>
>>> Could this process only the nodes which changed or does it really need to reprocess every node?
>> Only AddP, LoadP and StoreP (and narrow variants) are reprocessed. All other nodes are marked in vectSet _processed. There is check at the beginning of build_connection_graph().
>> I thought about using a separate worklist in new iterating code and populate it only with A/L/S nodes but I don't think it will help much.
>> And we need to reprocess all A/L/S nodes since we don't know which nodes will be affected by one change (the chain through deferred edges could be long and split/merge through Phis).
> 
> Sounds and looks good.
> 
> tom
> 
>> Thanks,
>> Vladimir
>>
>>> tom
>>> On Nov 2, 2010, at 3:27 PM, Vladimir Kozlov wrote:
>>>> http://cr.openjdk.java.net/~kvn/6991188/webrev.00
>>>>
>>>> Fixed 6991188: C2 Crashes while compiling method
>>>>
>>>> Construction of Connection Graph during EA relied on
>>>> DU relation corresponds to nodes index so that def
>>>> nodes processed before their uses. Move EA after
>>>> IGVN (for 6966411 fix broke that. Because of that
>>>> not all Graph edges will be created leading to
>>>> incorrect EA results and incorrect Ideal graph.
>>>>
>>>> Walk over interesting nodes (AddP, LoadP, StoreP)
>>>> several times until no new edges are created.
>>>> Set limit on iterations.
>>>>
>>>> Tested with assert in iterations bailout code and
>>>> calls to PhaseIdealLoop::verify() (removed from
>>>> final changes). Passed JPRT, full CTW, nsk.
>>>>
>