infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-01-28 12:09:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

8366888: C2: incorrect assertion predicate with short running long counted loop

Browse Source 
Co-authored-by: Christian Hagedorn <chagedorn@openjdk.org>
Reviewed-by: chagedorn, bmaillard
This commit is contained in:
Roland Westrelin 2025-11-25 13:00:07 +00:00
parent 49176e322b
commit 35f4a7410c
8 changed files with 266 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/hotspot/share/opto/loopTransform.cpp
View File

@ -1411,7 +1411,6 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
C->print_method(PHASE_BEFORE_PRE_MAIN_POST, 4, main_head);
Node *pre_header= main_head->in(LoopNode::EntryControl);
Node *init = main_head->init_trip();
Node *incr = main_end ->incr();
Node *limit = main_end ->limit();

38
src/hotspot/share/opto/loopnode.cpp
View File

@ -1162,13 +1162,16 @@ bool PhaseIdealLoop::create_loop_nest(IdealLoopTree* loop, Node_List &old_new) {
class CloneShortLoopPredicateVisitor : public PredicateVisitor {
ClonePredicateToTargetLoop _clone_predicate_to_loop;
PhaseIdealLoop* const _phase;
Node* const _new_init;
public:
CloneShortLoopPredicateVisitor(LoopNode* target_loop_head,
Node* new_init,
const NodeInSingleLoopBody &node_in_loop_body,
PhaseIdealLoop* phase)
: _clone_predicate_to_loop(target_loop_head, node_in_loop_body, phase),
_phase(phase) {
_phase(phase),
_new_init(new_init) {
}
NONCOPYABLE(CloneShortLoopPredicateVisitor);
@ -1180,11 +1183,32 @@ public:
}
void visit(const TemplateAssertionPredicate& template_assertion_predicate) override {
_clone_predicate_to_loop.clone_template_assertion_predicate(template_assertion_predicate);
_clone_predicate_to_loop.clone_template_assertion_predicate_and_replace_init(template_assertion_predicate, _new_init);
template_assertion_predicate.kill(_phase->igvn());
}
};
// For an int counted loop, try_make_short_running_loop() transforms the loop from:
// for (int = start; i < stop; i+= stride) { ... }
// to
// for (int = 0; i < stop - start; i+= stride) { ... }
// Template Assertion Predicates added so far were with an init value of start. They need to be updated with the new
// init value of 0 (otherwise when a template assertion predicate is turned into an initialized assertion predicate, it
// performs an incorrect check):
// zero
// init |
// | ===> OpaqueLoopInit init
// OpaqueLoopInit \ /
// AddI
//
Node* PhaseIdealLoop::new_assertion_predicate_opaque_init(Node* entry_control, Node* init, Node* int_zero) {
OpaqueLoopInitNode* new_opaque_init = new OpaqueLoopInitNode(C, int_zero);
register_new_node(new_opaque_init, entry_control);
Node* new_init = new AddINode(new_opaque_init, init);
register_new_node(new_init, entry_control);
return new_init;
}
// If the loop is either statically known to run for a small enough number of iterations or if profile data indicates
// that, we don't want an outer loop because the overhead of having an outer loop whose backedge is never taken, has a
// measurable cost. Furthermore, creating the loop nest usually causes one iteration of the loop to be peeled so
@ -1236,6 +1260,7 @@ bool PhaseIdealLoop::try_make_short_running_loop(IdealLoopTree* loop, jint strid
}
register_new_node(new_limit, entry_control);
Node* int_zero = intcon(0);
PhiNode* phi = head->phi()->as_Phi();
if (profile_short_running_loop) {
// Add a Short Running Long Loop Predicate. It's the first predicate in the predicate chain before entering a loop
@ -1261,9 +1286,11 @@ bool PhaseIdealLoop::try_make_short_running_loop(IdealLoopTree* loop, jint strid
if (!short_running_long_loop_predicate_block->has_parse_predicate()) { // already trapped
return false;
}
Node* new_init = new_assertion_predicate_opaque_init(entry_control, init, int_zero);
PredicateIterator predicate_iterator(entry_control);
NodeInSingleLoopBody node_in_short_loop_body(this, loop);
CloneShortLoopPredicateVisitor clone_short_loop_predicates_visitor(head, node_in_short_loop_body, this);
CloneShortLoopPredicateVisitor clone_short_loop_predicates_visitor(head, new_init, node_in_short_loop_body, this);
predicate_iterator.for_each(clone_short_loop_predicates_visitor);
entry_control = head->skip_strip_mined()->in(LoopNode::EntryControl);
@ -1311,6 +1338,10 @@ bool PhaseIdealLoop::try_make_short_running_loop(IdealLoopTree* loop, jint strid
register_new_node(new_limit, predicates.entry());
} else {
assert(bt == T_INT && known_short_running_loop, "only CountedLoop statically known to be short running");
PredicateIterator predicate_iterator(entry_control);
Node* new_init = new_assertion_predicate_opaque_init(entry_control, init, int_zero);
UpdateInitForTemplateAssertionPredicates update_init_for_template_assertion_predicates(new_init, this);
predicate_iterator.for_each(update_init_for_template_assertion_predicates);
}
IfNode* exit_test = head->loopexit();
@ -1320,7 +1351,6 @@ bool PhaseIdealLoop::try_make_short_running_loop(IdealLoopTree* loop, jint strid
register_new_node(new_limit, entry_control);
}
Node* int_zero = intcon(0);
if (stride_con < 0) {
new_limit = new SubINode(int_zero, new_limit);
register_new_node(new_limit, entry_control);

2
src/hotspot/share/opto/loopnode.hpp
View File

@ -1969,6 +1969,8 @@ public:
Node* ensure_node_and_inputs_are_above_pre_end(CountedLoopEndNode* pre_end, Node* node);
Node* new_assertion_predicate_opaque_init(Node* entry_control, Node* init, Node* int_zero);
bool try_make_short_running_loop(IdealLoopTree* loop, jint stride_con, const Node_List& range_checks, const uint iters_limit);
ConINode* intcon(jint i);

7
src/hotspot/share/opto/node.hpp
View File

@ -2176,7 +2176,10 @@ class BFSActions : public StackObj {
virtual bool is_target_node(Node* node) const = 0;
// Defines an action that should be taken when we visit a target node in the BFS traversal.
virtual void target_node_action(Node* target_node) = 0;
// To give more freedom, we pass the direct child node to the target node such that
// child->in(i) == target node. This allows to also directly replace the target node instead
// of only updating its inputs.
virtual void target_node_action(Node* child, uint i) = 0;
};
// Class to perform a BFS traversal on the data nodes from a given start node. The provided BFSActions guide which
@ -2198,7 +2201,7 @@ class DataNodeBFS : public StackObj {
Node* input = next->in(j);
if (_bfs_actions.is_target_node(input)) {
assert(_bfs_actions.should_visit(input), "must also pass node filter");
_bfs_actions.target_node_action(input);
_bfs_actions.target_node_action(next, j);
} else if (_bfs_actions.should_visit(input)) {
_nodes_to_visit.push(input);
}

75
src/hotspot/share/opto/predicates.cpp
View File

@ -198,12 +198,21 @@ TemplateAssertionPredicate TemplateAssertionPredicate::clone_and_replace_opaque_
Node* new_opaque_input,
CountedLoopNode* new_loop_node,
PhaseIdealLoop* phase) const {
DEBUG_ONLY(verify();)
OpaqueLoopInitNode* new_opaque_init = new OpaqueLoopInitNode(phase->C, new_opaque_input);
phase->register_new_node(new_opaque_init, new_control);
return clone_and_replace_init(new_control, new_opaque_init, new_loop_node, phase);
}
// Clone this Template Assertion Predicate and replace the old OpaqueLoopInit node with 'new_init'.
// Note: 'new_init' could also have the 'OpaqueLoopInit` as parent node further up.
TemplateAssertionPredicate TemplateAssertionPredicate::clone_and_replace_init(Node* new_control,
Node* new_init,
CountedLoopNode* new_loop_node,
PhaseIdealLoop* phase) const {
DEBUG_ONLY(verify();)
TemplateAssertionExpression template_assertion_expression(opaque_node(), phase);
OpaqueTemplateAssertionPredicateNode* new_opaque_node =
template_assertion_expression.clone_and_replace_init(new_control, new_opaque_init, new_loop_node);
template_assertion_expression.clone_and_replace_init(new_control, new_init, new_loop_node);
AssertionPredicateIfCreator assertion_predicate_if_creator(phase);
IfTrueNode* success_proj = assertion_predicate_if_creator.create_for_template(new_control, _if_node->Opcode(),
new_opaque_node,
@ -238,8 +247,40 @@ class ReplaceOpaqueStrideInput : public BFSActions {
return node->is_OpaqueLoopStride();
}
void target_node_action(Node* target_node) override {
_igvn.replace_input_of(target_node, 1, _new_opaque_stride_input);
void target_node_action(Node* child, uint i) override {
assert(child->in(i)->is_OpaqueLoopStride(), "must be OpaqueLoopStride");
_igvn.replace_input_of(child->in(i), 1, _new_opaque_stride_input);
}
};
// This class is used to replace the OpaqueLoopInitNode with a new node while leaving the other nodes
// unchanged.
class ReplaceOpaqueInitNode : public BFSActions {
Node* _new_opaque_init_node;
PhaseIterGVN& _igvn;
public:
ReplaceOpaqueInitNode(Node* new_opaque_init_node, PhaseIterGVN& igvn)
: _new_opaque_init_node(new_opaque_init_node),
_igvn(igvn) {}
NONCOPYABLE(ReplaceOpaqueInitNode);
void replace_for(OpaqueTemplateAssertionPredicateNode* opaque_node) {
DataNodeBFS bfs(*this);
bfs.run(opaque_node);
}
bool should_visit(Node* node) const override {
return TemplateAssertionExpressionNode::is_maybe_in_expression(node);
}
bool is_target_node(Node* node) const override {
return node->is_OpaqueLoopInit();
}
void target_node_action(Node* child, uint i) override {
assert(child->in(i)->is_OpaqueLoopInit(), "must be old OpaqueLoopInit");
_igvn.replace_input_of(child, i, _new_opaque_init_node);
}
};
@ -250,6 +291,13 @@ void TemplateAssertionPredicate::replace_opaque_stride_input(Node* new_stride, P
replace_opaque_stride_input.replace_for(opaque_node());
}
// Replace the OpaqueLoopInitNode with 'new_init' and leave the other nodes unchanged.
void TemplateAssertionPredicate::replace_opaque_init_node(Node* new_init, PhaseIterGVN& igvn) const {
DEBUG_ONLY(verify();)
ReplaceOpaqueInitNode replace_opaque_init_node(new_init, igvn);
replace_opaque_init_node.replace_for(opaque_node());
}
// Create a new Initialized Assertion Predicate from this template at the template success projection.
InitializedAssertionPredicate TemplateAssertionPredicate::initialize(PhaseIdealLoop* phase) const {
DEBUG_ONLY(verify();)
@ -308,7 +356,8 @@ class OpaqueLoopNodesVerifier : public BFSActions {
return node->is_Opaque1();
}
void target_node_action(Node* target_node) override {
void target_node_action(Node* child, uint i) override {
Node* target_node = child->in(i);
if (target_node->is_OpaqueLoopInit()) {
assert(!_found_init, "should only find one OpaqueLoopInitNode");
_found_init = true;
@ -1094,6 +1143,18 @@ void ClonePredicateToTargetLoop::clone_template_assertion_predicate(
_target_loop_predicate_chain.insert_predicate(cloned_template_assertion_predicate);
}
// Clones the provided Template Assertion Predicate to the head of the current predicate chain at the target loop and
// replaces the current OpaqueLoopInit with 'new_init'.
// Note: 'new_init' could also have the 'OpaqueLoopInit` as parent node further up.
void ClonePredicateToTargetLoop::clone_template_assertion_predicate_and_replace_init(
const TemplateAssertionPredicate& template_assertion_predicate, Node* new_init) {
TemplateAssertionPredicate cloned_template_assertion_predicate =
template_assertion_predicate.clone_and_replace_init(_old_target_loop_entry, new_init, _target_loop_head->as_CountedLoop(), _phase);
template_assertion_predicate.rewire_loop_data_dependencies(cloned_template_assertion_predicate.tail(),
_node_in_loop_body, _phase);
_target_loop_predicate_chain.insert_predicate(cloned_template_assertion_predicate);
}
CloneUnswitchedLoopPredicatesVisitor::CloneUnswitchedLoopPredicatesVisitor(
LoopNode* true_path_loop_head, LoopNode* false_path_loop_head,
const NodeInOriginalLoopBody& node_in_true_path_loop_body, const NodeInClonedLoopBody& node_in_false_path_loop_body,
@ -1182,6 +1243,10 @@ void UpdateStrideForAssertionPredicates::connect_initialized_assertion_predicate
}
}
void UpdateInitForTemplateAssertionPredicates::visit(const TemplateAssertionPredicate& template_assertion_predicate) {
template_assertion_predicate.replace_opaque_init_node(_new_init, _phase->igvn());
}
// Do the following to find and eliminate useless Parse and Template Assertion Predicates:
// 1. Mark all Parse and Template Assertion Predicates "maybe useful".
// 2. Walk through the loop tree and iterate over all Predicates above each loop head. All found Parse and Template

20
src/hotspot/share/opto/predicates.hpp
View File

@ -438,7 +438,10 @@ class TemplateAssertionPredicate : public Predicate {
TemplateAssertionPredicate clone(Node* new_control, CountedLoopNode* new_loop_node, PhaseIdealLoop* phase) const;
TemplateAssertionPredicate clone_and_replace_opaque_input(Node* new_control, Node* new_opaque_input,
CountedLoopNode* new_loop_node, PhaseIdealLoop* phase) const;
TemplateAssertionPredicate clone_and_replace_init(Node* new_control, Node* new_input,
CountedLoopNode* new_loop_node, PhaseIdealLoop* phase) const;
void replace_opaque_stride_input(Node* new_stride, PhaseIterGVN& igvn) const;
void replace_opaque_init_node(Node* new_init, PhaseIterGVN& igvn) const;
InitializedAssertionPredicate initialize(PhaseIdealLoop* phase) const;
void rewire_loop_data_dependencies(IfTrueNode* target_predicate, const NodeInLoopBody& data_in_loop_body,
const PhaseIdealLoop* phase) const;
@ -1228,6 +1231,7 @@ public:
}
void clone_template_assertion_predicate(const TemplateAssertionPredicate& template_assertion_predicate);
void clone_template_assertion_predicate_and_replace_init(const TemplateAssertionPredicate& template_assertion_predicate, Node* new_init);
};
// Visitor to clone Parse and Template Assertion Predicates from a loop to its unswitched true and false path loop.
@ -1300,6 +1304,22 @@ class UpdateStrideForAssertionPredicates : public PredicateVisitor {
void visit(const InitializedAssertionPredicate& initialized_assertion_predicate) override;
};
// This visitor replaces the OpaqueLoopInitNode for an Assertion Predicate with the expression passed as input.
class UpdateInitForTemplateAssertionPredicates : public PredicateVisitor {
Node* const _new_init;
PhaseIdealLoop* const _phase;
public:
UpdateInitForTemplateAssertionPredicates(Node* const new_init, PhaseIdealLoop* phase)
: _new_init(new_init),
_phase(phase) {}
NONCOPYABLE(UpdateInitForTemplateAssertionPredicates);
using PredicateVisitor::visit;
void visit(const TemplateAssertionPredicate& template_assertion_predicate) override;
};
// Eliminate all useless Parse and Template Assertion Predicates. They become useless when they can no longer be found
// from a loop head. We mark these useless to clean them up later during IGVN. A Predicate that is marked useless will
// no longer be visited by a PredicateVisitor.

81
test/hotspot/jtreg/compiler/longcountedloops/TestShortCountedLoopWithLongRCBadAssertPredicate.java Normal file
View File

@ -0,0 +1,81 @@
/*
* Copyright (c) 2025 IBM Corporation. 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 8366888
* @summary C2: incorrect assertion predicate with short running long counted loop
*
* @run main/othervm -XX:-BackgroundCompilation TestShortCountedLoopWithLongRCBadAssertPredicate
* @run main TestShortCountedLoopWithLongRCBadAssertPredicate
*/
import java.util.Objects;
public class TestShortCountedLoopWithLongRCBadAssertPredicate {
public static void main(String[] args) {
float[] floatArray = new float[1000];
for (int i = 0; i < 20_000; i++) {
test1(floatArray, 10000);
test2(floatArray, 10000);
test3(100, 1100, floatArray, 10000);
test4(999, 0, floatArray, 10000);
}
}
private static float test1(float[] floatArray, long longRange) {
float v = 0;
for (int i = 100; i < 1100; i++) {
v += floatArray[i - 100];
Objects.checkIndex(i, longRange);
}
return v;
}
private static float test2(float[] floatArray, long longRange) {
float v = 0;
for (int i = 999; i >= 0; i--) {
v += floatArray[i];
Objects.checkIndex(i, longRange);
}
return v;
}
private static float test3(int start, int stop, float[] floatArray, long longRange) {
float v = 0;
for (int i = start; i < stop; i++) {
v += floatArray[i - 100];
Objects.checkIndex(i, longRange);
}
return v;
}
private static float test4(int start, int stop, float[] floatArray, long longRange) {
float v = 0;
for (int i = start; i >= stop; i--) {
v += floatArray[i];
Objects.checkIndex(i, longRange);
}
return v;
}
}

54
test/hotspot/jtreg/compiler/longcountedloops/TestShortCountedLoopWithLongRCBadAssertPredicate2.java Normal file
View File

@ -0,0 +1,54 @@
/*
* Copyright (c) 2025, 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 8366888
* @summary C2: incorrect assertion predicate with short running long counted loop
*
* @run main/othervm -XX:CompileOnly=TestShortCountedLoopWithLongRCBadAssertPredicate2::test -Xbatch TestShortCountedLoopWithLongRCBadAssertPredicate2
* @run main/othervm TestShortCountedLoopWithLongRCBadAssertPredicate2
*/
import java.lang.foreign.MemorySegment;
import java.lang.foreign.ValueLayout;
public class TestShortCountedLoopWithLongRCBadAssertPredicate2 {
private static MemorySegment ms = MemorySegment.ofArray(new byte[80000]);
private static final ValueLayout.OfByte BYTE = ValueLayout.JAVA_BYTE;
static byte[] bArr = new byte[80000];
public static void main(String[] args) {
for (int i = 0; i < 10000; i++) {
test(i);
}
}
public static void test(int start) {
int end = start + 100;
int i = 0;
while (start < end) {
bArr[i++] = ms.get(BYTE, start++);
}
}
}