RFR(L): 8185556: [MVT] C2 compiler support for non-flattened value type fields

Tobias Hartmann tobias.hartmann at oracle.com
Wed Sep 13 13:20:37 UTC 2017


Hi,

please review the following patch:
https://bugs.openjdk.java.net/browse/JDK-8185556
http://cr.openjdk.java.net/~thartmann/8185556/webrev.00/

This change adds C2 support for non-flattened value type fields:
- when loading a non-flattened field, C2 now emits a null check and either uses the default value type or loads the 
value type from the oop
- we currently do *not* initialize the field if it's null
- most changes are in valuetypenode.cpp but since we now need to forward-propagate control after the null check, I had 
to update all user code as well
- no more code duplication in ValueType allocate and store methods
- had to change IdealKit and GraphKit to work after parsing to support optimizations in CheckCastPPNode::Ideal
- lots of refactoring and cleanups
- greatly improved the ValueTypeTestBench and found/fixed lots of bugs in the process (see below for details)

This change is based on interpreter support which was not yet integrated.

Here is a detailed explanation of all other (non-trivial) changes:

ciInstanceKlass.cpp
- moved _nof_declared_nonstatic_fields into ciValueKlass.cpp

ciValueKlass.cpp
- got completely rid of _field_index_map by using a _declared_nonstatic_fields array
- replaced wrapper methods by direct field array access and did lots of refactoring

callGenerator.cpp
- line 529: we need to check for return_type->is__Value() in the else branch because call->_tf might have been updated 
in CastPPNode::Ideal to a different type

castnode.cpp
- changed CheckCastPPNode::Ideal to use GraphKit for value type allocation and initialization because we need to emit GC 
barriers and complex control flow with non-flattened value type fields (and oop fields)
- accomplished this by small changes to the GraphKit to work with IGVN (see below) and building re-building the JVMState 
from the call
- changing "hook code" to detach and re-attach user nodes instead of new nodes was necessary to work with GraphKit and 
also simplified the code

cfgnode.cpp
- the PhiNode may merge different pointer types, we need to cast the type to BOTTOM
- to avoid an endless loop, we need to bail out in case of a constant Phi input

escape.cpp
- the tagged klass was recently updated to use a TypeRawPtr (see ValueTypeNode::tagged_klass()) but the code in line 
3186 was not updated

graphKit.cpp
- allowing to use the graphKit after parsing with IGVN
- avoid uncommon trap when reading an uninitialized non-flattened value type field
- changed code in GraphKit::set_results_for_java_call to execute make_slow_call_ex first to get right control edge when 
null-checking non-flattened value type fields

graphKit.hpp
- Added verification code to prevent worklist inconsistencies when using IGVN in GraphKit during incremental inlining

loopopts.cpp
- disabled workaround for JDK-8186716 [1] that avoids the assert but generates very inefficient code

macro.cpp
- line 2768: fix for "only applicable to compressed klass pointers" assert in oopDesc::klass_gap_offset_in_bytes()

parse1.cpp
- If we return from root in Parse::return_current, we need to make sure that all non-flattened value type fields are 
allocated

phaseX.cpp
- If we use the GraphKit with IGVN and _delay_transform, we may transform the same node multiple times. Don't use 
register_new_node_with_optimizer because it tries to set the type multiple times.

type.cpp
- support arrays for T_VALUETYPEPTR were not initialized
- we need to use BOTTOM type for non-flattened value type fields in in collect_value_fields
- in with_field_offset_and_offset we need to handle AddPNodes with an offset pointing to the beginning of the flattened 
value type array element (i.e., not pointing to a field)
- TypeValueTypePtr should not define empty() as false

valuetypenode.cpp
- make_scalar_in_safepoint now supports non-flattened, non-allocated value type fields by maintaining a worklist and 
adding SafePointScalarObjectNodes "recursively"
- added code to ValueTypeNode::make to update/propagate control for null checking non-flattened fields
- GVN version of ValueTypeBaseNode::store did not support oop fields (no GC barriers)
- completely removed the GVN version of ValueTypeBaseNode::allocate and ValueTypeBaseNode::store. We are now using the 
GraphKit version that correctly emits GC barriers and makes it easier to propagate the JVMState

sharedRuntime.cpp
- Skip assert if code is executed from compiler thread to avoid class loading assert

globalDefinitions.hpp
- T_VALUETYPEPTR_aelem_bytes was wrong

ValueTypeTestBench.java
- more runs with different flag combinations to increase coverage
- added MyValue2Inline and MyValue3Inline to increase nesting of value types
- marked slow tests with @Slow
- we now have the following configuration switches
   -DSkipSlow=true: Skip all tests marked with @Slow
   -DVerifyIR=true: Enable C2 IR verification
   -DVerifyVM=true: Enable additional verification flags defined in 'verifyFlags'
   -DTestlist=test1,test2: Only execute the specified tests
   -DPrintTimes: measure and print execution time of tests
   -DWarmup=X: set number of warmup iterations to X
- IR verification is currently disabled for the runs without field flattening
- Added -XX:ValueTypesBufferMaxMemory=0 to work around interpreter buffering bugs

All tests pass.

Thanks,
Tobias

[1] https://bugs.openjdk.java.net/browse/JDK-8186716


More information about the valhalla-dev mailing list