C2: add SafePointScalarObjectNodes from ValueTypeNodes to SafePointNodes without an allocation + propagate materialized value type in the graph

Tobias Hartmann tobias.hartmann at oracle.com
Tue Nov 8 08:00:20 UTC 2016


Hi Roland,

On 07.11.2016 17:54, Roland Westrelin wrote:
> 
> http://cr.openjdk.java.net/~roland/valhalla/scalar_at_safepoints/webrev.00/
> 
> In general we shouldn't need a temporary allocation to eliminate value
> type nodes at SafePoint nodes. 

Yes, I completely agree that we shouldn't emit a temporary allocation that is then removed anyway. Just used this as a straight forward implementation.

> Also, the temporary allocation currently
> happens at uncommon traps but there are other safepoints that are not
> currently handled. For instance, the first call to sumValue() in
> test14() has a reference to v that is not currently handled. This change
> creates SafePointScalarObjectNodes from ValueTypeNodes without a
> temporary allocation. This doesn't take care of a safepoint that
> references a value type that references another value type. I'll fix
> that once flattened value type support is integrated. I'll also remove
> the graphKit.cpp code that adds temporary allocations at uncommon traps
> later.

Right, I hit this problem with the tests for my flattened VT prototype as well. Good that you were able to fix this!

> I also noticed that test14() currently causes 2 allocations to be
> emitted. Same with test15(). The change to
> ValueTypeNode::store_to_memory() fixes that by creating a new
> ValueTypeNode that has a reference to the allocation and replacing the
> old one with the new one in the JVM state.

Your fix looks good to me, please go ahead and push. I'll then merge my flattened VT changes in.

Thanks,
Tobias

> Finally, I noticed WhiteBox.deoptimize() doesn't work properly. That
> will need to be fixed in jdk9 as well apparently (I can take care of
> it).
> 
> Roland.
> 


More information about the valhalla-dev mailing list