8307715: Integrate VectorMask/Shuffle with value/primitive classes #845
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. | ||
| * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. | ||
| * | ||
| * This code is free software; you can redistribute it and/or modify it | ||
|
|
@@ -151,6 +151,7 @@ bool InlineTypeNode::has_phi_inputs(Node* region) { | |
|
|
||
| // Merges 'this' with 'other' by updating the input PhiNodes added by 'clone_with_phis' | ||
| InlineTypeNode* InlineTypeNode::merge_with(PhaseGVN* gvn, const InlineTypeNode* other, int pnum, bool transform) { | ||
| assert(this->Opcode() == other->Opcode(), ""); | ||
| _is_buffered = _is_buffered && other->_is_buffered; | ||
| // Merge oop inputs | ||
| PhiNode* phi = get_oop()->as_Phi(); | ||
|
|
@@ -408,6 +409,11 @@ const TypePtr* InlineTypeNode::field_adr_type(Node* base, int offset, ciInstance | |
| } | ||
|
|
||
| void InlineTypeNode::load(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* holder, int holder_offset, DecoratorSet decorators) { | ||
| // FIXME: It is possible that "base" is a constant NULL_PTR, which is | ||
| // created by "gvn.zerocon(T_PRIMITIVE_OBJECT)". It has to do special | ||
| // handling for such cases. | ||
| assert(!kit->gvn().type(base)->maybe_null(), "the memory cannot be null"); | ||
|
|
||
| // Initialize the inline type by loading its field values from | ||
| // memory and adding the values as input edges to the node. | ||
| for (uint i = 0; i < field_count(); ++i) { | ||
|
|
@@ -425,7 +431,7 @@ void InlineTypeNode::load(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* | |
| const TypeOopPtr* oop_ptr = kit->gvn().type(base)->isa_oopptr(); | ||
| bool is_array = (oop_ptr->isa_aryptr() != NULL); | ||
| bool mismatched = (decorators & C2_MISMATCHED) != 0; | ||
| if (base->is_Con() && !is_array && !mismatched && ft->bundle_size() == 1) { | ||
| // If the oop to the inline type is constant (static final field), we can | ||
| // also treat the fields as constants because the inline type is immutable. | ||
| ciObject* constant_oop = oop_ptr->const_oop(); | ||
|
|
@@ -440,12 +446,6 @@ void InlineTypeNode::load(GraphKit* kit, Node* base, Node* ptr, ciInstanceKlass* | |
| ft = con_type->inline_klass(); | ||
| null_free = true; | ||
| } | ||
|
Comment on lines
-443
to
-448
There was a problem hiding this comment. It may be needed for constant oops (gvn.zerocon(T_PRIMITIVE_OBJECT)) as created on following line. There was a problem hiding this comment. The reason that I change this here is I met another issue when the constant oops is There was a problem hiding this comment. It will not be a problem with constant oops or even default values which are value_mirrors, but gvn.zerocon(T_PRIMITIVE_OBJECT) will create a ConP node representing a NULL_PTR, any address commutation w.r.t. base is will result into illegal address. We should keep multi-field handling in both the control paths and add an extract check for ConP of TypePtr::NULL_PTR in if condition, it will handle all the cases. There was a problem hiding this comment. Make sense to me. But if the constant oop is There was a problem hiding this comment. Do you have a test when the constant oop's type is NULL_PTR? I can prepare a fixing and testing. Thanks! There was a problem hiding this comment. Phi is still carrying an abstract VectorMask type where as one of its input is concrete mask which is why C2 injects InlineTypeNode for it. There was a problem hiding this comment. Yes, so do you know how this PhiNode is generated and what it is used for? From the log, I can see the two inputs of Phi are all from There was a problem hiding this comment. I also met several issues due to the VectorAPI abstract classes and its concrete vector classes. As I see, in compiler, sometimes the argument/return type is calculated from the sigunature type of the method, but sometimes it is calculated based on the speculative type which use the profiling info as a reference. Following is part of my fixing for such cases: There was a problem hiding this comment.
Do not consider empty output list of the PhiNode, this dump was taken from debugger during a reverse debugging step, so it may appear its not getting used. All the signatures of top level VectorAPIs methods always deal in abstract types, its only due to aggressive in-lining boxes (concrete vector/mask/shuffle) gets exposed. We have currently disabled value scalarization for vectors, please also add mask/shuffle to that list https://github.com/openjdk/valhalla/blob/lworld%2Bvector/src/hotspot/share/oops/inlineKlass.cpp#L341 There was a problem hiding this comment. Have added by extending |
||
| } else { | ||
| // Load field value from memory | ||
| BasicType bt = type2field[ft->basic_type()]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per JEP Value classes (primitve and value) are implicitly final. Also both oop and InlineTypeNode should always be of same type. Do you see a case where a cast should be applied to oop but InlineTypeNode remain sacrosanct ?
Only one I can think off is when InlineTypeNode was created using make_default / make_uninitialized and ciInstanceKlass was uninitialized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the relative API is
VectorMask.fromArray(), which the InlineTypeNode type is the concrete value class (e.g.Byte128Mask.class), while the receiver type is its super class (e.g.VectorMask.class). I think it is generated betweenVectorBoxand its relative inline method call.