Skip to content

Commit

Permalink
8315003: [lworld] C2: Implement full support for JDK-8287061 and JDK-…
Browse files Browse the repository at this point in the history
…8316991

Reviewed-by: thartmann
  • Loading branch information
chhagedorn committed Oct 10, 2024
1 parent 1a935cb commit 0ae00ea
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 21 deletions.
11 changes: 11 additions & 0 deletions src/hotspot/share/opto/cfgnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2763,6 +2763,17 @@ bool PhiNode::can_push_inline_types_down(PhaseGVN* phase, const bool can_reshape
return true;
}

#ifdef ASSERT
bool PhiNode::can_push_inline_types_down(PhaseGVN* phase) {
if (!can_be_inline_type()) {
return false;
}

ciInlineKlass* inline_klass;
return can_push_inline_types_down(phase, true, inline_klass);
}
#endif // ASSERT

static int compare_types(const Type* const& e1, const Type* const& e2) {
return (intptr_t)e1 - (intptr_t)e2;
}
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/opto/cfgnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ class PhiNode : public TypeNode {
}

Node* try_push_inline_types_down(PhaseGVN* phase, bool can_reshape);
DEBUG_ONLY(bool can_push_inline_types_down(PhaseGVN* phase);)

virtual const Type* Value(PhaseGVN* phase) const;
virtual Node* Identity(PhaseGVN* phase);
Expand Down
46 changes: 32 additions & 14 deletions src/hotspot/share/opto/escape.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "opto/cfgnode.hpp"
#include "opto/compile.hpp"
#include "opto/escape.hpp"
#include "opto/inlinetypenode.hpp"
#include "opto/macro.hpp"
#include "opto/locknode.hpp"
#include "opto/phaseX.hpp"
Expand Down Expand Up @@ -567,8 +568,7 @@ bool ConnectionGraph::can_reduce_check_users(Node* n, uint nesting) const {
} else if (nesting > 0) {
NOT_PRODUCT(if (TraceReduceAllocationMerges) tty->print_cr("Can NOT reduce Phi %d on invocation %d. Unsupported user %s at nesting level %d.", n->_idx, _invocation, use->Name(), nesting);)
return false;
// TODO 8315003 Re-enable
} else if (use->is_CastPP() && false) {
} else if (use->is_CastPP()) {
const Type* cast_t = _igvn->type(use);
if (cast_t == nullptr || cast_t->make_ptr()->isa_instptr() == nullptr) {
#ifndef PRODUCT
Expand All @@ -586,18 +586,23 @@ bool ConnectionGraph::can_reduce_check_users(Node* n, uint nesting) const {
// CmpP/N used by the If controlling the cast.
if (use->in(0)->is_IfTrue() || use->in(0)->is_IfFalse()) {
Node* iff = use->in(0)->in(0);
if (iff->Opcode() == Op_If && iff->in(1)->is_Bool() && iff->in(1)->in(1)->is_Cmp()) {
// We may have Opaque4 node between If and Bool nodes.
// Bail out in such case - we need to preserve Opaque4 for correct
// processing predicates after loop opts.
bool can_reduce = (iff->Opcode() == Op_If) && iff->in(1)->is_Bool() && iff->in(1)->in(1)->is_Cmp();
if (can_reduce) {
Node* iff_cmp = iff->in(1)->in(1);
int opc = iff_cmp->Opcode();
if ((opc == Op_CmpP || opc == Op_CmpN) && !can_reduce_cmp(n, iff_cmp)) {
can_reduce = (opc == Op_CmpP || opc == Op_CmpN) && can_reduce_cmp(n, iff_cmp);
}
if (!can_reduce) {
#ifndef PRODUCT
if (TraceReduceAllocationMerges) {
tty->print_cr("Can NOT reduce Phi %d on invocation %d. CastPP %d doesn't have simple control.", n->_idx, _invocation, use->_idx);
n->dump(5);
}
#endif
return false;
if (TraceReduceAllocationMerges) {
tty->print_cr("Can NOT reduce Phi %d on invocation %d. CastPP %d doesn't have simple control.", n->_idx, _invocation, use->_idx);
n->dump(5);
}
#endif
return false;
}
}
}
Expand Down Expand Up @@ -1250,12 +1255,16 @@ bool ConnectionGraph::reduce_phi_on_safepoints_helper(Node* ophi, Node* cast, No

AllocateNode* alloc = ptn->ideal_node()->as_Allocate();
Unique_Node_List value_worklist;
SafePointScalarObjectNode* sobj = mexp.create_scalarized_object_description(alloc, sfpt, &value_worklist);
// TODO 8315003 Remove this bailout
if (value_worklist.size() != 0) {
return false;
#ifdef ASSERT
const Type* res_type = alloc->result_cast()->bottom_type();
if (res_type->is_inlinetypeptr() && !Compile::current()->has_circular_inline_type()) {
PhiNode* phi = ophi->as_Phi();
assert(!ophi->as_Phi()->can_push_inline_types_down(_igvn), "missed earlier scalarization opportunity");
}
#endif
SafePointScalarObjectNode* sobj = mexp.create_scalarized_object_description(alloc, sfpt, &value_worklist);
if (sobj == nullptr) {
_compile->record_failure(C2Compiler::retry_no_reduce_allocation_merges());
return false;
}

Expand All @@ -1266,6 +1275,15 @@ bool ConnectionGraph::reduce_phi_on_safepoints_helper(Node* ophi, Node* cast, No

// Register the scalarized object as a candidate for reallocation
smerge->add_req(sobj);

// Scalarize inline types that were added to the safepoint.
// Don't allow linking a constant oop (if available) for flat array elements
// because Deoptimization::reassign_flat_array_elements needs field values.
const bool allow_oop = !merge_t->is_flat();
for (uint j = 0; j < value_worklist.size(); ++j) {
InlineTypeNode* vt = value_worklist.at(j)->as_InlineType();
vt->make_scalar_in_safepoints(_igvn, allow_oop);
}
}

// Replaces debug information references to "original_sfpt_parent" in "sfpt" with references to "smerge"
Expand Down
2 changes: 0 additions & 2 deletions src/hotspot/share/opto/macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -866,8 +866,6 @@ SafePointScalarObjectNode* PhaseMacroExpand::create_scalarized_object_descriptio
}

if (res->bottom_type()->is_inlinetypeptr()) {
// TODO 8315003 This starts to fail after JDK-8316991. Fix and re-enable.
// assert(C->has_circular_inline_type(), "non-circular inline types should have been replaced earlier");
// Nullable inline types have an IsInit field which is added to the safepoint when scalarizing them (see
// InlineTypeNode::make_scalar_in_safepoint()). When having circular inline types, we stop scalarizing at depth 1
// to avoid an endless recursion. Therefore, we do not have a SafePointScalarObjectNode node here, yet.
Expand Down
2 changes: 0 additions & 2 deletions test/hotspot/jtreg/ProblemList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ compiler/floatingpoint/TestSubnormalDouble.java 8317810 generic-i586

compiler/startup/StartupOutput.java 8326615 generic-x64

compiler/c2/irTests/scalarReplacement/AllocationMergesTests.java 8315003 generic-all

compiler/codecache/CodeCacheFullCountTest.java 8332954 generic-all

#############################################################################
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright (c) 2024, 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.
*/

/*
* @test
* @bug 8315003
* @summary Test that removing allocation merges of non-value and value object at EA is working properly.
* @library /test/lib /
* @enablePreview
* @run main compiler.valhalla.inlinetypes.TestAllocationMergeAndFolding
*/

package compiler.valhalla.inlinetypes;

import compiler.lib.ir_framework.*;
import jdk.test.lib.Utils;

import java.util.Random;

public class TestAllocationMergeAndFolding {
private static final Random RANDOM = Utils.getRandomInstance();

public static void main(String[] args) {
InlineTypes.getFramework()
.addScenarios(InlineTypes.DEFAULT_SCENARIOS)
.addScenarios(new Scenario(6, "--enable-preview", "-XX:-UseCompressedOops"))
.addScenarios(new Scenario(7, "--enable-preview", "-XX:+UseCompressedOops"))
.start();
}

@Test
@IR(failOn = IRNode.ALLOC)
static int test(boolean flag) {
Object o;
if (flag) {
o = new V(34);
} else {
o = new Object();
}
dontInline(); // Not inlined and thus we have a safepoint where keep phi(o) = [V, Object].

// 'o' escapes as store to 'f'. However, 'f' does not escape and can be removed. As a result, we can also remove
// the allocations in both branches with EA after JDK-8287061. Since V has an inline type field v2, we put it
// on a list to scalarize it as well. The improved allocation merge was disabled in Valhalla but is now enabled
// and fixed with JDK-8315003.
Foo f = new Foo(o);
return f.i;
}

@DontInline
static void dontInline() {
}

@Run(test = "test")
static void run() {
test(RANDOM.nextBoolean());
}

static class Foo {
Object o;
int i;

Foo(Object o) {
this.o = o;
}
}

static value class V {
int i;
V2 v2;

V(int i) {
this.i = i;
this.v2 = new V2();
}
}

static value class V2 {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,6 @@ public void test11_verifier() {
}

// Test loop with uncommon trap referencing a value object
// TODO 8315003 Re-enable
/*
@Test
@IR(applyIf = {"FlatArrayElementMaxSize", "= -1"},
counts = {SCOBJ, ">= 1", LOAD, "<= 12"}) // TODO 8227588 (loads should be removed)
Expand Down Expand Up @@ -319,7 +317,6 @@ public void test12_verifier(RunInfo info) {
long result = test12(info.isWarmUp());
Asserts.assertEQ(result, info.isWarmUp() ? rL + (1000 * rI) : ((Math.abs(rI) % 10) + 1) * hash());
}
*/

// Test loop with uncommon trap referencing a value object
@Test
Expand Down

0 comments on commit 0ae00ea

Please sign in to comment.