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