[lworld+vector] RFR: 8304310: Initial compilers and runtime handling for multifield backed vectors. [v14]
Xiaohong Gong
xgong at openjdk.org
Wed May 10 02:28:48 UTC 2023
On Tue, 9 May 2023 19:13:25 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:
>> Please find below the summary of changes included with the patch:
>>
>> a) _ci Model:_
>> oops model creates separate fieldDescriptor/FieldInfo for each scalar field of a multifield bundle, root field is marked as multifield_base and non - root (synthetic) fileds carry a multifield flag. To ease vector IR construction ci only exposes base multifield and ciType holding bundle size to compilers.
>>
>> b) _C1 compiler:_
>> Special handling to copy entire multifield bundle during withfield bytecode processing.
>>
>> c) _C2 compiler:_
>> VectorBox becomes a derivative of InlineType IR node, changes in vector box/unbox expansions. Preventing scalarization of Vector arguments and return values.
>>
>> d) _Runtime:_
>> Changes in object re-construction during deoptimization since now concrete vectors have multifield based payloads. Payload (VectorPayloadMF) is a primitive class object which gets fully flattened within its parent/holding instance i.e. a concrete vector, special handling has been added for non-flattened case where payload is allocated over heap and concrete vector payload field is assigned its reference.
>>
>> e) Added type specific multifield payloads to ease vector IR creation by C2, this will ensure bundle type and C2 IR type are compatible.
>>
>> Scope of current changes is limited to Vector types only, shuffles and masks are still backed by array based payloads. This PoC patch has been validated over vector only tests. More robust validation will be done in due course.
>>
>> More information can be found at following link
>> [https://cr.openjdk.org/~jbhateja/vectorIntrinsics/Valhalla/lworld%2Bvector.pptx](https://cr.openjdk.org/~jbhateja/vectorIntrinsics/Valhalla/lworld%2Bvector.pptx)
>>
>> Please share your feedback and suggestions.
>>
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>
> Review comments resolutions.
src/hotspot/share/ci/ciField.hpp line 192:
> 190: bool is_null_free () const { return _is_null_free; }
> 191:
> 192:
style: please remove this new blank line.
src/hotspot/share/classfile/vmClassMacros.hpp line 197:
> 195: do_klass(vector_VectorPayloadMF_klass, jdk_internal_vm_vector_VectorPayloadMF ) \
> 196: do_klass(vector_VectorPayloadMF64B_klass, jdk_internal_vm_vector_VectorPayloadMF64B ) \
> 197: do_klass(vector_VectorPayloadMF128B_klass, jdk_internal_vm_vector_VectorPayloadMF128B ) \
Suggestion:
do_klass(vector_VectorPayloadMF64B_klass, jdk_internal_vm_vector_VectorPayloadMF64B ) \
do_klass(vector_VectorPayloadMF128B_klass, jdk_internal_vm_vector_VectorPayloadMF128B ) \
src/hotspot/share/classfile/vmSymbols.hpp line 124:
> 122: template(mfield_name, "mfield") \
> 123: template(ETYPE_name, "ETYPE") \
> 124: template(VLENGTH_name, "VLENGTH") \
style: suggest to align with line-120.
Suggestion:
template(jdk_internal_vm_vector_VectorPayloadMF512D, "jdk/internal/vm/vector/VectorSupport$VectorPayloadMF512D") \
template(payload_name, "payload") \
template(mfield_name, "mfield") \
template(ETYPE_name, "ETYPE") \
template(VLENGTH_name, "VLENGTH") \
src/hotspot/share/classfile/vmSymbols.hpp line 298:
> 296: template(vector_VectorPayloadMF64B_signature, "Qjdk/internal/vm/vector/VectorSupport$VectorPayloadMF64B;") \
> 297: template(vector_VectorPayloadMF128B_signature, "Qjdk/internal/vm/vector/VectorSupport$VectorPayloadMF128B;") \
> 298: template(vector_VectorPayloadMF256B_signature, "Qjdk/internal/vm/vector/VectorSupport$VectorPayloadMF256B;") \
style: ditto, alignment please.
src/hotspot/share/opto/cfgnode.cpp line 2704:
> 2702: ciInlineKlass* payload = vk->declared_nonstatic_field_at(0)->type()->as_inline_klass();
> 2703:
> 2704: Node* payload_oop = payload->is_initialized() ? InlineTypeNode::default_oop(*igvn, payload) : igvn->zerocon(T_PRIMITIVE_OBJECT);
`payload_oop` is not used and can be removed?
src/hotspot/share/opto/inlinetypenode.cpp line 36:
> 34: #include "opto/phaseX.hpp"
> 35: #include "opto/vectornode.hpp"
> 36:
style: one more blank line.
src/hotspot/share/opto/inlinetypenode.hpp line 31:
> 29: #include "opto/loopnode.hpp"
> 30: #include "opto/node.hpp"
> 31: #include "opto/matcher.hpp"
style: include reorder
src/hotspot/share/opto/inlinetypenode.hpp line 61:
> 59: void expand_input_edges(ciInlineKlass * vk);
> 60:
> 61:
style: two more blank lines
src/hotspot/share/opto/inlinetypenode.hpp line 127:
> 125: bool is_multifield_base(uint index) const;
> 126: int secondary_fields_count(uint index) const;
> 127: bool is_multifield() const;
`is_multifield()` is not implemented and used?
src/hotspot/share/opto/vector.cpp line 34:
> 32: #include "opto/vector.hpp"
> 33: #include "utilities/macros.hpp"
> 34: #include "prims/vectorSupport.hpp"
style: include reorder
src/hotspot/share/opto/vectornode.hpp line 31:
> 29: #include "opto/memnode.hpp"
> 30: #include "opto/node.hpp"
> 31: #include "opto/inlinetypenode.hpp"
ditto
src/hotspot/share/prims/vectorSupport.cpp line 29:
> 27: #include "classfile/vmClasses.hpp"
> 28: #include "classfile/vmSymbols.hpp"
> 29: #include "classfile/vmClassMacros.hpp"
style: ditto
src/hotspot/share/runtime/fieldDescriptor.hpp line 98:
> 96: inline jbyte multifield_index() const;
> 97: inline int secondary_fields_count(int base_idx) const;
> 98:
style: one more blank line
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189281241
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189283354
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189283691
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189284324
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189285035
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189285461
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189286201
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189286327
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189287204
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189290492
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189291378
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189291604
PR Review Comment: https://git.openjdk.org/valhalla/pull/833#discussion_r1189291994
More information about the valhalla-dev
mailing list