Request for reviews (S): 6991188: C2 Crashes while compiling method
Y. S. Ramakrishna
y.s.ramakrishna at oracle.com
Wed Nov 3 09:59:53 PDT 2010
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?)
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:
>>> + // Possible infinit build_connection_graph loop.
>>> 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.
>>> On Nov 2, 2010, at 3:27 PM, Vladimir Kozlov wrote:
>>>> 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.
More information about the hotspot-compiler-dev