Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions src/hotspot/share/ci/ciInstanceKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -391,17 +391,23 @@ bool ciInstanceKlass::contains_field_offset(int offset) {
return get_instanceKlass()->contains_field_offset(offset);
}

// ------------------------------------------------------------------
// ciInstanceKlass::get_non_static_field_by_offset
ciField* ciInstanceKlass::get_non_static_field_by_offset(const int field_offset) {
for (int i = 0, len = nof_nonstatic_fields(); i < len; i++) {
ciField* field = _nonstatic_fields->at(i);
int field_off = field->offset_in_bytes();
if (field_off == field_offset)
return field;
}
return nullptr;
}

// ------------------------------------------------------------------
// ciInstanceKlass::get_field_by_offset
ciField* ciInstanceKlass::get_field_by_offset(int field_offset, bool is_static) {
if (!is_static) {
for (int i = 0, len = nof_nonstatic_fields(); i < len; i++) {
ciField* field = _nonstatic_fields->at(i);
int field_off = field->offset_in_bytes();
if (field_off == field_offset)
return field;
}
return nullptr;
return get_non_static_field_by_offset(field_offset);
}
VM_ENTRY_MARK;
InstanceKlass* k = get_instanceKlass();
Expand All @@ -427,6 +433,33 @@ ciField* ciInstanceKlass::get_field_by_name(ciSymbol* name, ciSymbol* signature,
return field;
}

// ------------------------------------------------------------------
// ciInstanceKlass::get_field_type_by_offset
//
// This is essentially a shortcut for:
// get_field_by_offset(field_offset, is_static)->layout_type()
// except this does not require allocating memory for a new ciField
BasicType ciInstanceKlass::get_field_type_by_offset(const int field_offset, const bool is_static) {
if (!is_static) {
ciField* field = get_non_static_field_by_offset(field_offset);
return field != nullptr ? field->layout_type() : T_ILLEGAL;
}

// Avoid allocating a new ciField by obtaining the field type directly
VM_ENTRY_MARK;
InstanceKlass* k = get_instanceKlass();
fieldDescriptor fd;
if (!k->find_field_from_offset(field_offset, is_static, &fd)) {
return T_ILLEGAL;
}

// Reproduce the behavior of ciField::layout_type
BasicType field_type = fd.field_type();
if (is_reference_type(field_type)) {
return T_OBJECT;
}
return type2field[make(field_type)->basic_type()];
}

// ------------------------------------------------------------------
// ciInstanceKlass::compute_nonstatic_fields
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/ci/ciInstanceKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class ciInstanceKlass : public ciKlass {
bool compute_injected_fields_helper();
void compute_transitive_interfaces();

ciField* get_non_static_field_by_offset(int field_offset);

protected:
ciInstanceKlass(Klass* k);
ciInstanceKlass(ciSymbol* name, jobject loader);
Expand Down Expand Up @@ -204,6 +206,7 @@ class ciInstanceKlass : public ciKlass {
ciInstanceKlass* get_canonical_holder(int offset);
ciField* get_field_by_offset(int field_offset, bool is_static);
ciField* get_field_by_name(ciSymbol* name, ciSymbol* signature, bool is_static);
BasicType get_field_type_by_offset(int field_offset, bool is_static);

// total number of nonstatic fields (including inherited):
int nof_nonstatic_fields() {
Expand Down
12 changes: 5 additions & 7 deletions src/hotspot/share/opto/type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3478,23 +3478,21 @@ TypeOopPtr::TypeOopPtr(TYPES t, PTR ptr, ciKlass* k, const TypeInterfaces* inter
} else if (klass() == ciEnv::current()->Class_klass() &&
_offset >= InstanceMirrorKlass::offset_of_static_fields()) {
// Static fields
ciField* field = nullptr;
BasicType basic_elem_type = T_ILLEGAL;
if (const_oop() != nullptr) {
ciInstanceKlass* k = const_oop()->as_instance()->java_lang_Class_klass()->as_instance_klass();
field = k->get_field_by_offset(_offset, true);
basic_elem_type = k->get_field_type_by_offset(_offset, true);
}
if (field != nullptr) {
BasicType basic_elem_type = field->layout_type();
if (basic_elem_type != T_ILLEGAL) {
_is_ptr_to_narrowoop = UseCompressedOops && ::is_reference_type(basic_elem_type);
} else {
// unsafe access
_is_ptr_to_narrowoop = UseCompressedOops;
}
} else {
// Instance fields which contains a compressed oop references.
ciField* field = ik->get_field_by_offset(_offset, false);
if (field != nullptr) {
BasicType basic_elem_type = field->layout_type();
BasicType basic_elem_type = ik->get_field_type_by_offset(_offset, false);
if (basic_elem_type != T_ILLEGAL) {
_is_ptr_to_narrowoop = UseCompressedOops && ::is_reference_type(basic_elem_type);
} else if (klass()->equals(ciEnv::current()->Object_klass())) {
// Compile::find_alias_type() cast exactness on all types to verify
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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.
*/

package compiler.loopopts;

/**
* @test
* @bug 8366990
* @summary Loop optimizations verification results in hitting the memory limit.
* This is caused by the high number of verification passes triggered
* in PhaseIdealLoop::split_if_with_blocks_post and repetitive memory
* allocations while building the ideal Loop tree in preparation for
* the verification.
*
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
* -XX:CompileCommand=compileonly,compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit::test
* -XX:CompileCommand=memlimit,compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit::test,100M~crash
* -XX:-TieredCompilation -Xcomp -XX:PerMethodTrapLimit=0
* -XX:+StressLoopPeeling -XX:+VerifyLoopOptimizations
* -XX:StressSeed=1870557292
Copy link
Member

@chhagedorn chhagedorn Oct 10, 2025

Choose a reason for hiding this comment

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

I suggest to remove the stress seed since it might not trigger anymore in later builds. Usually, we add a run with a fixed stress seed and one without but since this test requires to do just some verification work, I would suggest to not add two runs but only one without fixed seed.

Another question: How close are we to hit the default the memory limit with this test? With your fix it probably consumes not much memory anymore. I therefore suggest to add MemLimit as additional flag with a much smaller value to be sure that your fix works as expected (you might need to check how low we can choose the limit to not run into problems in higher tiers).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to reduce the test further using a memory limit of 100M (approximately 10 times less than the default) and a shorter timeout with creduce. Compilation of the new test method with a fast debug build now takes an average of 1.22 s over 100 runs according to -XX:+CITime.
With the decrease compilation time I think it now reasonable to have two runs (one with the stress seed, one without). Let me know if you think otherwise!

* compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit
* @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -XX:+UnlockDiagnosticVMOptions
* -XX:CompileCommand=compileonly,compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit::test
* -XX:CompileCommand=memlimit,compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit::test,100M~crash
* -XX:-TieredCompilation -Xcomp -XX:PerMethodTrapLimit=0
* -XX:+StressLoopPeeling -XX:+VerifyLoopOptimizations
* compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit
* @run main compiler.loopopts.TestVerifyLoopOptimizationsHitsMemLimit
*
*/

public class TestVerifyLoopOptimizationsHitsMemLimit {
final int a = 400;
int b;
float c;
static double d;
static byte f;
long g[];
volatile int h[];

void test() {
int j, k = 2, l, o[] = new int[a];
short m = 10492;
for (j = 1;; ++j) {
l = 1;
do {
g[j] = l;
switch (j) {
case 45:
o[1] = b;
case 163:
case 62:
case 72:
case 319:
h[1] -= k;
case 109:
case 47:
case 91:
case 68:
case 162:
case 76:
case 60:
case 66:
case 83:
d = m;
case 2314:
f = (byte) c;
}
} while (++l < 4);
}
}

public static void main(String[] args) {
try {
TestVerifyLoopOptimizationsHitsMemLimit test = new TestVerifyLoopOptimizationsHitsMemLimit();
test.test();
throw new RuntimeException("Expected a NPE for uninitialized array");
} catch (NullPointerException e) {
// expected
}
}
}