Skip to content

Commit

Permalink
8313672: C2: PhaseCCP does not correctly track analysis dependencies …
Browse files Browse the repository at this point in the history
…involving shift, convert, and mask

Reviewed-by: epeter, rcastanedalo, thartmann
  • Loading branch information
dlunde authored and robcasloz committed Nov 15, 2023
1 parent fbe1937 commit bad6999
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 26 deletions.
22 changes: 0 additions & 22 deletions src/hotspot/share/opto/castnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,28 +80,6 @@ class ConstraintCastNode: public TypeNode {

Node* optimize_integer_cast(PhaseGVN* phase, BasicType bt);

// Visit all non-cast uses of the node, bypassing ConstraintCasts.
// Pattern: this (-> ConstraintCast)* -> non_cast
// In other words: find all non_cast nodes such that
// non_cast->uncast() == this.
template <typename Callback>
static void visit_uncasted_uses(const Node* n, Callback callback) {
ResourceMark rm;
Unique_Node_List internals;
internals.push((Node*)n); // start traversal
for (uint j = 0; j < internals.size(); ++j) {
Node* internal = internals.at(j); // for every internal
for (DUIterator_Fast kmax, k = internal->fast_outs(kmax); k < kmax; k++) {
Node* internal_use = internal->fast_out(k);
if (internal_use->is_ConstraintCast()) {
internals.push(internal_use); // traverse this cast also
} else {
callback(internal_use);
}
}
}
}

bool higher_equal_types(PhaseGVN* phase, const Node* other) const;

int extra_types_count() const {
Expand Down
36 changes: 36 additions & 0 deletions src/hotspot/share/opto/node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,13 @@ class Node {
// Set control or add control as precedence edge
void ensure_control_or_add_prec(Node* c);

// Visit boundary uses of the node and apply a callback function for each.
// Recursively traverse uses, stopping and applying the callback when
// reaching a boundary node, defined by is_boundary. Note: the function
// definition appears after the complete type definition of Node_List.
template <typename Callback, typename Check>
void visit_uses(Callback callback, Check is_boundary) const;

//----------------- Code Generation

// Ideal register class for Matching. Zero means unmatched instruction
Expand Down Expand Up @@ -1628,6 +1635,35 @@ class Node_List : public Node_Array {
void dump_simple() const;
};

// Definition must appear after complete type definition of Node_List
template <typename Callback, typename Check>
void Node::visit_uses(Callback callback, Check is_boundary) const {
ResourceMark rm;
VectorSet visited;
Node_List worklist;

// The initial worklist consists of the direct uses
for (DUIterator_Fast kmax, k = fast_outs(kmax); k < kmax; k++) {
Node* out = fast_out(k);
if (!visited.test_set(out->_idx)) { worklist.push(out); }
}

while (worklist.size() > 0) {
Node* use = worklist.pop();
// Apply callback on boundary nodes
if (is_boundary(use)) {
callback(use);
} else {
// Not a boundary node, continue search
for (DUIterator_Fast kmax, k = use->fast_outs(kmax); k < kmax; k++) {
Node* out = use->fast_out(k);
if (!visited.test_set(out->_idx)) { worklist.push(out); }
}
}
}
}


//------------------------------Unique_Node_List-------------------------------
class Unique_Node_List : public Node_List {
friend class VMStructs;
Expand Down
13 changes: 9 additions & 4 deletions src/hotspot/share/opto/phaseX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,9 @@ void PhaseIterGVN::add_users_to_worklist( Node *n ) {
_worklist.push(n);
}
};
ConstraintCastNode::visit_uncasted_uses(use, push_the_uses_to_worklist);
auto is_boundary = [](Node* n){ return !n->is_ConstraintCast(); };
use->visit_uses(push_the_uses_to_worklist, is_boundary);

}
// If changed LShift inputs, check RShift users for useless sign-ext
if( use_op == Op_LShiftI ) {
Expand Down Expand Up @@ -1832,7 +1834,7 @@ void PhaseCCP::verify_analyze(Unique_Node_List& worklist_verify) {
// We should either make sure that these nodes are properly added back to the CCP worklist
// in PhaseCCP::push_child_nodes_to_worklist() to update their type or add an exception
// in the verification code above if that is not possible for some reason (like Load nodes).
assert(!failure, "Missed optimization opportunity in PhaseCCP");
assert(!failure, "PhaseCCP not at fixpoint: analysis result may be unsound.");
}
#endif

Expand Down Expand Up @@ -1974,7 +1976,7 @@ void PhaseCCP::push_load_barrier(Unique_Node_List& worklist, const BarrierSetC2*

// AndI/L::Value() optimizes patterns similar to (v << 2) & 3 to zero if they are bitwise disjoint.
// Add the AndI/L nodes back to the worklist to re-apply Value() in case the shift value changed.
// Pattern: parent -> LShift (use) -> ConstraintCast* -> And
// Pattern: parent -> LShift (use) -> (ConstraintCast | ConvI2L)* -> And
void PhaseCCP::push_and(Unique_Node_List& worklist, const Node* parent, const Node* use) const {
uint use_op = use->Opcode();
if ((use_op == Op_LShiftI || use_op == Op_LShiftL)
Expand All @@ -1985,7 +1987,10 @@ void PhaseCCP::push_and(Unique_Node_List& worklist, const Node* parent, const No
push_if_not_bottom_type(worklist, n);
}
};
ConstraintCastNode::visit_uncasted_uses(use, push_and_uses_to_worklist);
auto is_boundary = [](Node* n) {
return !(n->is_ConstraintCast() || n->Opcode() == Op_ConvI2L);
};
use->visit_uses(push_and_uses_to_worklist, is_boundary);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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 8313672
* @summary Test CCP notification for value update of AndL through LShiftI and
* ConvI2L.
* @run main/othervm -XX:+UnlockDiagnosticVMOptions
* -XX:RepeatCompilation=20 -XX:-TieredCompilation
* -XX:+StressIGVN -Xcomp
* -XX:CompileCommand=compileonly,compiler.ccp.TestShiftConvertAndNotification::test
* compiler.ccp.TestShiftConvertAndNotification
*/

/*
* @test
* @bug 8313672
* @summary Test CCP notification for value update of AndL through LShiftI and
* ConvI2L (no flags).
* @run driver compiler.ccp.TestShiftConvertAndNotification
*
*/

package compiler.ccp;

public class TestShiftConvertAndNotification {
static long instanceCount;
static void test() {
int i, i1 = 7;
for (i = 7; i < 45; i++) {
instanceCount = i;
instanceCount &= i1 * i << i * Math.max(instanceCount, instanceCount);
switch (i % 2) {
case 8:
i1 = 0;
}
}
}
public static void main(String[] strArr) {
for (int i = 0; i < 20_000; i++)
test();
}
}

1 comment on commit bad6999

@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.