Skip to content

Commit

Permalink
8316105: C2: Back to back Parse Predicates from different loops but w…
Browse files Browse the repository at this point in the history
…ith same deopt reason are wrongly grouped together

Reviewed-by: roland, thartmann, kvn
  • Loading branch information
chhagedorn committed Sep 21, 2023
1 parent 1749ba2 commit ca47f5f
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 2 deletions.
17 changes: 16 additions & 1 deletion src/hotspot/share/opto/predicates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Deoptimization::DeoptReason RuntimePredicate::uncommon_trap_reason(IfProjNode* i
}

bool RuntimePredicate::is_success_proj(Node* node, Deoptimization::DeoptReason deopt_reason) {
if (node->is_IfProj()) {
if (node->is_IfProj() && !node->in(0)->is_ParsePredicate()) {
return deopt_reason == uncommon_trap_reason(node->as_IfProj());
} else {
return false;
Expand Down Expand Up @@ -108,6 +108,21 @@ ParsePredicateNode* ParsePredicateIterator::next() {
return _parse_predicates.at(_current_index++);
}

#ifdef ASSERT
// Check that the block has at most one Parse Predicate and that we only find Regular Predicate nodes (i.e. IfProj,
// If, or RangeCheck nodes).
void PredicateBlock::verify_block() {
Node* next = _parse_predicate.entry(); // Skip unique Parse Predicate of this block if present
while (next != _entry) {
assert(!next->is_ParsePredicate(), "can only have one Parse Predicate in a block");
const int opcode = next->Opcode();
assert(next->is_IfProj() || opcode == Op_If || opcode == Op_RangeCheck,
"Regular Predicates consist of an IfProj and an If or RangeCheck node");
next = next->in(0);
}
}
#endif // ASSERT

// Walk over all Regular Predicates of this block (if any) and return the first node not belonging to the block
// anymore (i.e. entry to the first Regular Predicate in this block if any or `regular_predicate_proj` otherwise).
Node* PredicateBlock::skip_regular_predicates(Node* regular_predicate_proj, Deoptimization::DeoptReason deopt_reason) {
Expand Down
5 changes: 4 additions & 1 deletion src/hotspot/share/opto/predicates.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,14 @@ class PredicateBlock : public StackObj {
Node* _entry;

static Node* skip_regular_predicates(Node* regular_predicate_proj, Deoptimization::DeoptReason deopt_reason);
DEBUG_ONLY(void verify_block();)

public:
PredicateBlock(Node* predicate_proj, Deoptimization::DeoptReason deopt_reason)
: _parse_predicate(predicate_proj, deopt_reason),
_entry(skip_regular_predicates(_parse_predicate.entry(), deopt_reason)) {}
_entry(skip_regular_predicates(_parse_predicate.entry(), deopt_reason)) {
DEBUG_ONLY(verify_block();)
}

// Returns the control input node into this Regular Predicate block. This is either:
// - The control input to the first If node in the block representing a Runtime Predicate if there is at least one
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright (c) 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
* 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 8316105
* @library /test/lib
* @summary Test that back to back Parse Predicates with same deopt reason are not grouped together
* @run main/othervm -Xbatch compiler.predicates.TestBackToBackParsePredicates
*/

package compiler.predicates;

import jdk.test.lib.Asserts;

public class TestBackToBackParsePredicates {
static long lFld;

public static void main(String[] strArr2) {
for (int i = -350; i <= 0; i++) {
lFld = 30;
test(i);
check();
}
lFld = 30;
test(1);
check();
}

// Inlined
static void foo() {
for (int i12 = 1; i12 < 5; i12++) { // Loop A
lFld += 1; // StoreL
}
}

static void test(int x) {
foo();

// After fully unrolling loop A and after next round of IGVN:
// We wrongly treat two back to back Loop Limit Check Parse Predicates as single Predicate Block. We therefore
// keep the Loop Parse Predicate of loop A:
//
// Loop Parse Predicate (of A)
// Loop Limit Check Parse Predicate (of A) |
// -> StoreL of lFld pinned here | Wrongly treated as single Predicate Block
// Loop Limit Check Parse Predicate (of B) |
for (int i = 7; i < 212; i++) { // Loop B
for (int j = 1; j < 80; j++) {
switch (x % 8) {
case 0:
case 2:
break;
case 6:
case 7:
}
}
}
}

static void check() {
Asserts.assertEQ(34L, lFld);
}
}

1 comment on commit ca47f5f

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.