Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8268553: [lworld] CI must return secondary mirror when accessing CONSTANT_Class with Q-signature #442

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -697,7 +697,15 @@ ciConstant ciEnv::get_constant_by_index_impl(const constantPoolHandle& cpool,
}
assert (klass->is_instance_klass() || klass->is_array_klass(),
"must be an instance or array klass ");
return ciConstant(T_OBJECT, klass->java_mirror());
if (tag.is_unresolved_klass()) {
Copy link
Member

@TobiHartmann TobiHartmann Jul 9, 2021

Choose a reason for hiding this comment

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

This is too strict because klass->is_loaded() can be true even if tag.is_unresolved_klass(). This leads to C2 adding uncommon traps to trigger resolution in the interpreter. However, I've observed cases were the interpreter never resolved the constant and tag.is_unresolved_klass() remained true, leading to continuous deoptimizations in C2 compiled code. I'm not sure if that is expected behavior or if there is a bug in the interpreter but the CI code should be fixed in any case. I'll do that with JDK-8267932.

Copy link
Collaborator Author

@fparain fparain Jul 19, 2021

Choose a reason for hiding this comment

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

Do you have an example where the interpreter never resolves the entry?

return ciConstant(T_OBJECT, get_unloaded_klass_mirror(klass));
} else {
if (tag.is_Qdescriptor_klass()) {
return ciConstant(T_OBJECT, klass->as_inline_klass()->val_mirror());
} else {
return ciConstant(T_OBJECT, klass->java_mirror());
}
}
} else if (tag.is_method_type()) {
// must execute Java code to link this CP entry into cache[i].f1
ciSymbol* signature = get_symbol(cpool->method_type_signature_at(index));
@@ -140,3 +140,7 @@ address ciInlineKlass::unpack_handler() const {
InlineKlass* ciInlineKlass::get_InlineKlass() const {
GUARDED_VM_ENTRY(return to_InlineKlass();)
}

ciInstance* ciInlineKlass::val_mirror() {
GUARDED_VM_ENTRY(return CURRENT_ENV->get_instance(to_InlineKlass()->val_mirror());)
}
@@ -90,6 +90,7 @@ class ciInlineKlass : public ciInstanceKlass {
address pack_handler() const;
address unpack_handler() const;
InlineKlass* get_InlineKlass() const;
ciInstance* val_mirror();
};

#endif // SHARE_VM_CI_CIINLINEKLASS_HPP