Request for reviews (M): 6890673: Eliminate allocations immediately after EA
vladimir.kozlov at oracle.com
Tue Nov 15 14:48:59 PST 2011
Thank you, Tom
I filed RFE 7112269: cleanup remove_dead_region() usage
I agree that it should be called and checked for TOP in
PhaseGVN::transform_no_reclaim() and PhaseIterGVN::transform_old() to be
consistent for all ideal nodes.
Tom Rodriguez wrote:
> On Nov 14, 2011, at 6:28 PM, Vladimir Kozlov wrote:
>> 6890673: Eliminate allocations immediately after EA
>> Currently non-escaping allocations and corresponding locks are eliminated during macro nodes expansion after all ideal and loop optimizations which may prevent to do more aggressive (loop and split-if) optimizations.
>> Try to eliminate allocations and related locks immediately after escape analysis. Move elimination code into new separate method PhaseMacroExpand::eliminate_macro_nodes().
>> The main problem was that SafePointScalarObject may be referenced by several safepoints which were cloned, for example, during loop unswitch. As result current checks that SPSO should be pinned and at the same block as safepoint failed. Nothing is wrong to have one SPSO node be referenced by several cloned safepoints since SPSO contains only class information and index of safepoin's debug info edge which should stay the same for cloned safepoints. The solution is to removed pinned attribute from SPSO node, use Root as control edge and add hash(), cmp() to not allow value-numbering of these nodes.
>> Removed SPSO asserts in block.cpp and the rest is mostly code style clean up.
>> Locks eliminated by "lock coarsening" optimization are treated differently during elimination since corresponding outer locks should stay. Such locks marked by "_coarsened" flag. EA should clean this flag for locks which could be eliminated because corresponding object does not escape.
>> Don't bail out from build_and_optimize() until dead code is eliminated when it is called before EA even if there are no loops or there are only irreducible loops.
>> Added missing (in(0)->is_top()) checks after remove_dead_region() calls.
> Should this idiom:
> if( in(0) && remove_dead_region(phase, can_reshape) ) return this;
> // Don't bother trying to transform a dead node
> if( in(0) && in(0)->is_top() ) return NULL;
> be handled in a more systematic way instead of being sprinkled about? There are many Ideal functions that consist only of the first part which says that me that it's really part of the code transform loop instead of being a responsibility of Ideal itself. Maybe that's for a more broad cleanup in the future.
> Otherwise it looks good.
More information about the hotspot-compiler-dev