Skip to content
Permalink
Browse files
8247923: [lworld] Empty inline types break calling convention optimiz…
…ation
  • Loading branch information
TobiHartmann committed Jun 24, 2020
1 parent d615a33 commit 45400b1ebd69691011223596543bce6acd9f1366
Showing 10 changed files with 141 additions and 24 deletions.
@@ -76,14 +76,23 @@ bool ciValueKlass::flatten_array() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->flatten_array();)
}

// Can this value type be scalarized?
bool ciValueKlass::is_scalarizable() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->is_scalarizable();)
}

// Can this value type be passed as multiple values?
bool ciValueKlass::can_be_passed_as_fields() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->can_be_passed_as_fields();)
}

// Can this value type be returned as multiple values?
bool ciValueKlass::can_be_returned_as_fields() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->can_be_returned_as_fields();)
}

// Can this value type be scalarized?
bool ciValueKlass::is_scalarizable() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->is_scalarizable();)
bool ciValueKlass::is_empty() const {
GUARDED_VM_ENTRY(return to_ValueKlass()->is_empty_inline_type();)
}

// When passing a value type's fields as arguments, count the number
@@ -78,8 +78,10 @@ class ciValueKlass : public ciInstanceKlass {
int field_index_by_offset(int offset);

bool flatten_array() const;
bool can_be_returned_as_fields() const;
bool is_scalarizable() const;
bool can_be_passed_as_fields() const;
bool can_be_returned_as_fields() const;
bool is_empty() const;
int value_arg_slots();
int default_value_offset() const;
ciInstance* default_value_instance() const;
@@ -323,7 +323,7 @@ void ValueKlass::initialize_calling_convention(TRAPS) {
// Because the pack and unpack handler addresses need to be loadable from generated code,
// they are stored at a fixed offset in the klass metadata. Since value type klasses do
// not have a vtable, the vtable offset is used to store these addresses.
if (is_scalarizable() && (InlineTypeReturnedAsFields || InlineTypePassFieldsAsArgs)) {
if (InlineTypeReturnedAsFields || InlineTypePassFieldsAsArgs) {
ResourceMark rm;
GrowableArray<SigEntry> sig_vk;
int nb_fields = collect_fields(&sig_vk);
@@ -332,8 +332,7 @@ void ValueKlass::initialize_calling_convention(TRAPS) {
for (int i = 0; i < sig_vk.length(); i++) {
extended_sig->at_put(i, sig_vk.at(i));
}

if (InlineTypeReturnedAsFields) {
if (can_be_returned_as_fields(/* init= */ true)) {
nb_fields++;
BasicType* sig_bt = NEW_RESOURCE_ARRAY(BasicType, nb_fields);
sig_bt[0] = T_METADATA;
@@ -353,8 +352,13 @@ void ValueKlass::initialize_calling_convention(TRAPS) {
*((address*)adr_pack_handler_jobject()) = buffered_blob->pack_fields_jobject();
*((address*)adr_unpack_handler()) = buffered_blob->unpack_fields();
assert(CodeCache::find_blob(pack_handler()) == buffered_blob, "lost track of blob");
assert(can_be_returned_as_fields(), "sanity");
}
}
if (!can_be_returned_as_fields() && !can_be_passed_as_fields()) {
MetadataFactory::free_array<SigEntry>(class_loader_data(), extended_sig);
assert(return_regs() == NULL, "sanity");
}
}
}

@@ -389,9 +393,14 @@ bool ValueKlass::is_scalarizable() const {
return ScalarizeInlineTypes;
}

// Can this value type be passed as multiple values?
bool ValueKlass::can_be_passed_as_fields() const {
return InlineTypePassFieldsAsArgs && is_scalarizable() && !is_empty_inline_type();
}

// Can this value type be returned as multiple values?
bool ValueKlass::can_be_returned_as_fields() const {
return return_regs() != NULL;
bool ValueKlass::can_be_returned_as_fields(bool init) const {
return InlineTypeReturnedAsFields && is_scalarizable() && !is_empty_inline_type() && (init || return_regs() != NULL);
}

// Create handles for all oop fields returned in registers that are going to be live across a safepoint
@@ -250,7 +250,8 @@ class ValueKlass: public InstanceKlass {
return *((Array<VMRegPair>**)adr_return_regs());
}
bool is_scalarizable() const;
bool can_be_returned_as_fields() const;
bool can_be_passed_as_fields() const;
bool can_be_returned_as_fields(bool init = false) const;
void save_oop_fields(const RegisterMap& map, GrowableArray<Handle>& handles) const;
void restore_oop_results(RegisterMap& map, GrowableArray<Handle>& handles) const;
oop realloc_result(const RegisterMap& reg_map, const GrowableArray<Handle>& handles, TRAPS);
@@ -2037,7 +2037,7 @@ const TypeTuple *TypeTuple::make_domain(ciMethod* method, bool vt_fields_as_args
const Type** field_array = fields(arg_cnt);
if (!method->is_static()) {
ciInstanceKlass* recv = method->holder();
if (vt_fields_as_args && recv->is_valuetype() && recv->as_value_klass()->is_scalarizable()) {
if (vt_fields_as_args && recv->is_valuetype() && recv->as_value_klass()->can_be_passed_as_fields()) {
collect_value_fields(recv->as_value_klass(), field_array, pos, sig_cc);
} else {
field_array[pos++] = get_const_type(recv)->join_speculative(TypePtr::NOTNULL);
@@ -2076,7 +2076,7 @@ const TypeTuple *TypeTuple::make_domain(ciMethod* method, bool vt_fields_as_args
break;
case T_VALUETYPE: {
bool never_null = sig->is_never_null_at(i);
if (vt_fields_as_args && type->as_value_klass()->is_scalarizable() && never_null) {
if (vt_fields_as_args && type->as_value_klass()->can_be_passed_as_fields() && never_null) {
is_flattened = true;
collect_value_fields(type->as_value_klass(), field_array, pos, sig_cc);
} else {
@@ -495,7 +495,8 @@ Node* ValueTypeBaseNode::allocate_fields(GraphKit* kit) {

ValueTypeNode* ValueTypeNode::make_uninitialized(PhaseGVN& gvn, ciValueKlass* vk) {
// Create a new ValueTypeNode with uninitialized values and NULL oop
return new ValueTypeNode(vk, gvn.zerocon(T_VALUETYPE));
Node* oop = vk->is_empty() ? default_oop(gvn, vk) : gvn.zerocon(T_VALUETYPE);
return new ValueTypeNode(vk, oop);
}

Node* ValueTypeNode::default_oop(PhaseGVN& gvn, ciValueKlass* vk) {
@@ -1142,8 +1142,9 @@ Handle SharedRuntime::find_callee_info_helper(JavaThread* thread,
THROW_(vmSymbols::java_lang_NoSuchMethodException(), nullHandle);
}
}
if (!caller_is_c1 && callee->has_scalarized_args() && callee->method_holder()->is_inline_klass()) {
// If the receiver is a value type that is passed as fields, no oop is available.
if (!caller_is_c1 && callee->has_scalarized_args() && callee->method_holder()->is_inline_klass() &&
ValueKlass::cast(callee->method_holder())->can_be_passed_as_fields()) {
// If the receiver is an inline type that is passed as fields, no oop is available
// Resolve the call without receiver null checking.
assert(attached_method.not_null() && !attached_method->is_abstract(), "must have non-abstract attached method");
if (bc == Bytecodes::_invokeinterface) {
@@ -1285,7 +1286,8 @@ bool SharedRuntime::resolve_sub_helper_internal(methodHandle callee_method, cons

if (is_virtual) {
Klass* receiver_klass = NULL;
if (InlineTypePassFieldsAsArgs && !caller_is_c1 && callee_method->method_holder()->is_inline_klass()) {
if (!caller_is_c1 && callee_method->has_scalarized_args() && callee_method->method_holder()->is_inline_klass() &&
ValueKlass::cast(callee_method->method_holder())->can_be_passed_as_fields()) {
// If the receiver is an inline type that is passed as fields, no oop is available
receiver_klass = callee_method->method_holder();
} else {
@@ -2746,7 +2748,7 @@ int CompiledEntrySignature::compute_scalarized_cc(GrowableArray<SigEntry>*& sig_
InstanceKlass* holder = _method->method_holder();
sig_cc = new GrowableArray<SigEntry>(_method->size_of_parameters());
if (!_method->is_static()) {
if (holder->is_inline_klass() && scalar_receiver && ValueKlass::cast(holder)->is_scalarizable()) {
if (holder->is_inline_klass() && scalar_receiver && ValueKlass::cast(holder)->can_be_passed_as_fields()) {
sig_cc->appendAll(ValueKlass::cast(holder)->extended_sig());
} else {
SigEntry::add_entry(sig_cc, T_OBJECT);
@@ -2756,7 +2758,7 @@ int CompiledEntrySignature::compute_scalarized_cc(GrowableArray<SigEntry>*& sig_
for (SignatureStream ss(_method->signature()); !ss.at_return_type(); ss.next()) {
if (ss.type() == T_VALUETYPE) {
ValueKlass* vk = ss.as_value_klass(holder);
if (vk->is_scalarizable()) {
if (vk->can_be_passed_as_fields()) {
sig_cc->appendAll(vk->extended_sig());
} else {
SigEntry::add_entry(sig_cc, T_OBJECT);
@@ -2833,7 +2835,7 @@ CodeOffsets::Entries CompiledEntrySignature::c1_value_ro_entry_type() const {
void CompiledEntrySignature::compute_calling_conventions() {
// Get the (non-scalarized) signature and check for value type arguments
if (!_method->is_static()) {
if (_method->method_holder()->is_inline_klass() && ValueKlass::cast(_method->method_holder())->is_scalarizable()) {
if (_method->method_holder()->is_inline_klass() && ValueKlass::cast(_method->method_holder())->can_be_passed_as_fields()) {
_has_value_recv = true;
_num_value_args++;
}
@@ -2842,14 +2844,14 @@ void CompiledEntrySignature::compute_calling_conventions() {
for (SignatureStream ss(_method->signature()); !ss.at_return_type(); ss.next()) {
BasicType bt = ss.type();
if (bt == T_VALUETYPE) {
if (ss.as_value_klass(_method->method_holder())->is_scalarizable()) {
if (ss.as_value_klass(_method->method_holder())->can_be_passed_as_fields()) {
_num_value_args++;
}
bt = T_OBJECT;
}
SigEntry::add_entry(_sig, bt);
}
if (_method->is_abstract() && !(InlineTypePassFieldsAsArgs && has_value_arg())) {
if (_method->is_abstract() && !has_value_arg()) {
return;
}

@@ -2865,7 +2867,7 @@ void CompiledEntrySignature::compute_calling_conventions() {
_args_on_stack_cc = _args_on_stack;
_args_on_stack_cc_ro = _args_on_stack;

if (InlineTypePassFieldsAsArgs && has_value_arg() && !_method->is_native()) {
if (has_value_arg() && !_method->is_native()) {
_args_on_stack_cc = compute_scalarized_cc(_sig_cc, _regs_cc, /* scalar_receiver = */ true);

_sig_cc_ro = _sig_cc;
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2020, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package compiler.valhalla.valuetypes;

public final inline class MyValueEmpty extends MyAbstract {
public long hash() { return 0; }

public MyValueEmpty copy(MyValueEmpty other) { return other; }
}
@@ -33,7 +33,7 @@
* @summary Test value type calling convention optimizations
* @library /testlibrary /test/lib /compiler/whitebox /
* @requires (os.simpleArch == "x64" | os.simpleArch == "aarch64")
* @compile TestCallingConvention.java
* @compile -XDallowEmptyValues TestCallingConvention.java
* @run driver ClassFileInstaller sun.hotspot.WhiteBox jdk.test.lib.Platform
* @run main/othervm/timeout=300 -Xbootclasspath/a:. -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
* -XX:+UnlockExperimentalVMOptions -XX:+WhiteBoxAPI
@@ -786,4 +786,17 @@ public void test37_verifier(boolean warmup) throws Throwable {
int res = test37(vt);
Asserts.assertEQ(res, rI);
}

// Test passing/returning an empty inline type
@Test(failOn = ALLOC + LOAD + STORE + TRAP)
public MyValueEmpty test38(MyValueEmpty vt) {
return vt.copy(vt);
}

@DontCompile
public void test38_verifier(boolean warmup) {
MyValueEmpty vt = new MyValueEmpty();
MyValueEmpty res = test38(vt);
Asserts.assertEQ(res, vt);
}
}
@@ -36,7 +36,7 @@
* @modules java.base/jdk.experimental.value
* @library /testlibrary /test/lib /compiler/whitebox /
* @requires (os.simpleArch == "x64" | os.simpleArch == "aarch64")
* @compile TestLWorld.java
* @compile -XDallowEmptyValues TestLWorld.java
* @run driver ClassFileInstaller sun.hotspot.WhiteBox jdk.test.lib.Platform
* @run main/othervm/timeout=300 -Xbootclasspath/a:. -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
* -XX:+UnlockExperimentalVMOptions -XX:+WhiteBoxAPI
@@ -3241,4 +3241,54 @@ public void test115_verifier(boolean warmup) {
long res = test115();
Asserts.assertEquals(res, 5*rL);
}

static MyValueEmpty fEmpty1;
static MyValueEmpty.ref fEmpty2 = MyValueEmpty.default;
MyValueEmpty fEmpty3;
MyValueEmpty.ref fEmpty4 = MyValueEmpty.default;

// Test fields loads/stores with empty inline types
@Test(failOn = ALLOC + ALLOC_G + LOAD + STORE + TRAP)
public void test116() {
fEmpty1 = fEmpty4;
fEmpty2 = fEmpty1;
fEmpty3 = fEmpty2;
fEmpty4 = fEmpty3;
}

@DontCompile
public void test116_verifier(boolean warmup) {
test116();
Asserts.assertEquals(fEmpty1, fEmpty2);
Asserts.assertEquals(fEmpty2, fEmpty3);
Asserts.assertEquals(fEmpty3, fEmpty4);
}

// Test array loads/stores with empty inline types
@Test(failOn = ALLOC + ALLOC_G)
public MyValueEmpty test117(MyValueEmpty[] arr1, MyValueEmpty.ref[] arr2) {
arr1[0] = arr2[0];
arr2[0] = new MyValueEmpty();
return arr1[0];
}

@DontCompile
public void test117_verifier(boolean warmup) {
MyValueEmpty[] arr1 = new MyValueEmpty[]{MyValueEmpty.default};
MyValueEmpty res = test117(arr1, arr1);
Asserts.assertEquals(res, MyValueEmpty.default);
Asserts.assertEquals(arr1[0], MyValueEmpty.default);
}

// Test acmp with empty inline types
@Test(failOn = ALLOC + ALLOC_G)
public boolean test118(MyValueEmpty v1, MyValueEmpty.ref v2, Object o1) {
return (v1 == v2) && (v2 == o1);
}

@DontCompile
public void test118_verifier(boolean warmup) {
boolean res = test118(MyValueEmpty.default, MyValueEmpty.default, new MyValueEmpty());
Asserts.assertTrue(res);
}
}

0 comments on commit 45400b1

Please sign in to comment.