C2: add SafePointScalarObjectNodes from ValueTypeNodes to SafePointNodes without an allocation + propagate materialized value type in the graph
tobias.hartmann at oracle.com
Tue Nov 8 08:00:20 UTC 2016
On 07.11.2016 17:54, Roland Westrelin wrote:
> 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
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.
> 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
More information about the valhalla-dev