Skip to content

Commit

Permalink
8276064: CheckCastPP with raw oop input floats below a safepoint
Browse files Browse the repository at this point in the history
Backport-of: cd9c688bfce36e4b2d37dd68dd8031f197b9eddc
  • Loading branch information
GoeLin committed Jan 9, 2023
1 parent cf6c041 commit d784aae
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 8 deletions.
24 changes: 20 additions & 4 deletions src/hotspot/share/opto/buildOopMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,26 @@ OopMap *OopFlow::build_oop_map( Node *n, int max_reg, PhaseRegAlloc *regalloc, i
} else {
// Other - some reaching non-oop value
#ifdef ASSERT
if( t->isa_rawptr() && C->cfg()->_raw_oops.member(def) ) {
def->dump();
n->dump();
assert(false, "there should be a oop in OopMap instead of a live raw oop at safepoint");
if (t->isa_rawptr()) {
ResourceMark rm;
Unique_Node_List worklist;
worklist.push(def);
for (uint i = 0; i < worklist.size(); i++) {
Node* m = worklist.at(i);
if (C->cfg()->_raw_oops.member(m)) {
def->dump();
m->dump();
n->dump();
assert(false, "there should be an oop in OopMap instead of a live raw oop at safepoint");
}
// Check users as well because def might be spilled
for (DUIterator_Fast jmax, j = m->fast_outs(jmax); j < jmax; j++) {
Node* u = m->fast_out(j);
if ((u->is_SpillCopy() && u->in(1) == m) || u->is_Phi()) {
worklist.push(u);
}
}
}
}
#endif
}
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/share/opto/loopnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5430,6 +5430,11 @@ void PhaseIdealLoop::build_loop_late_post_work(Node *n, bool pinned) {
}
}
}
// Don't extend live ranges of raw oops
if (least != early && n->is_ConstraintCast() && n->in(1)->bottom_type()->isa_rawptr() &&
!n->bottom_type()->isa_rawptr()) {
least = early;
}

#ifdef ASSERT
// If verifying, verify that 'verify_me' has a legal location
Expand Down
4 changes: 0 additions & 4 deletions src/hotspot/share/opto/loopopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1467,16 +1467,12 @@ bool PhaseIdealLoop::safe_for_if_replacement(const Node* dom) const {
// like various versions of induction variable+offset. Clone the
// computation per usage to allow it to sink out of the loop.
void PhaseIdealLoop::try_sink_out_of_loop(Node* n) {
bool is_raw_to_oop_cast = n->is_ConstraintCast() &&
n->in(1)->bottom_type()->isa_rawptr() &&
!n->bottom_type()->isa_rawptr();
if (has_ctrl(n) &&
!n->is_Phi() &&
!n->is_Bool() &&
!n->is_Proj() &&
!n->is_MergeMem() &&
!n->is_CMove() &&
!is_raw_to_oop_cast && // don't extend live ranges of raw oops
n->Opcode() != Op_Opaque4 &&
!n->is_Type()) {
Node *n_ctrl = get_ctrl(n);
Expand Down
125 changes: 125 additions & 0 deletions test/hotspot/jtreg/compiler/vectorapi/TestRawOopAtSafepoint.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (c) 2022, 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.vectorapi;

import jdk.incubator.vector.IntVector;
import jdk.test.lib.Asserts;

/*
* @test
* @bug 8276064 8271600
* @summary Verify that CheckCastPPs with raw oop inputs are not floating below a safepoint.
* @library /test/lib
* @modules jdk.incubator.vector
* @run main/othervm -XX:-TieredCompilation -Xbatch
* -XX:CompileCommand=compileonly,compiler.vectorapi.TestRawOopAtSafepoint::test*
* -XX:CompileCommand=dontinline,compiler.vectorapi.TestRawOopAtSafepoint::safepoint
* compiler.vectorapi.TestRawOopAtSafepoint
*/
public class TestRawOopAtSafepoint {

static int iFld = 42;

public static void safepoint(boolean gc) {
if (gc) {
// Give the GC a chance to move the IntVector object on the heap
// and thus invalidate the oop if it's not in the oopMap.
System.gc();
}
}

// Loop unswitching moves a CheckCastPP out of a loop such that the raw oop
// input crosses a safepoint. We then SIGSEGV after the GC moved the IntVector
// object when deferencing the now stale oop.
public static IntVector test1(boolean flag, boolean gc) {
IntVector vector = null;
for (int i = 0; i < 100; i++) {
// Trigger loop unswitching
if (flag) {
iFld++;
}
// Allocate an IntVector that will be scalarized in the
// safepoint debug info but not in the return.
vector = IntVector.zero(IntVector.SPECIES_MAX);
safepoint((i == 99) && gc);
}
return vector;
}

// Same as test1 but we hit an assert in OopFlow::build_oop_map instead.
public static IntVector test2(boolean flag, boolean gc) {
for (int i = 0; i < 100; i++) {
// Trigger loop unswitching
if (flag) {
iFld++;
}
IntVector vector = IntVector.zero(IntVector.SPECIES_MAX);
safepoint((i == 99) && gc);
if (flag) {
return vector;
}
}
return IntVector.zero(IntVector.SPECIES_MAX);
}

// Same as test1 but PhaseIdealLoop::try_sink_out_of_loop moves the CheckCastPP.
public static IntVector test3(boolean flag, boolean gc) {
IntVector vector = null;
for (int i = 0; i < 10; i++) {
vector = IntVector.zero(IntVector.SPECIES_MAX);
safepoint((i == 9) && gc);
}
return vector;
}

// Same as test2 but PhaseIdealLoop::try_sink_out_of_loop moves the CheckCastPP.
public static IntVector test4(boolean flag, boolean gc) {
IntVector vector = null;
for (int i = 0; i < 2; i++) {
vector = IntVector.zero(IntVector.SPECIES_MAX);
safepoint((i == 9) && gc);
}
return vector;
}

public static void main(String[] args) {
int sum = 0;
for (int i = 0; i < 15_000; ++i) {
boolean flag = ((i % 2) == 0);
boolean gc = (i > 14_500);
IntVector vector1 = test1(flag, gc);
sum += vector1.lane(0);

IntVector vector2 = test2(flag, gc);
sum += vector2.lane(0);

IntVector vector3 = test3(flag, gc);
sum += vector3.lane(0);

IntVector vector4 = test4(flag, gc);
sum += vector4.lane(0);
}
Asserts.assertEQ(sum, 0);
}
}

1 comment on commit d784aae

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