Skip to content

Commit

Permalink
8335221: Some C2 intrinsics incorrectly assume that type argument is …
Browse files Browse the repository at this point in the history
…compile-time constant

Reviewed-by: thartmann
Backport-of: 166f9d9
  • Loading branch information
Vladimir Kozlov committed Jul 2, 2024
1 parent 272d11a commit b6d0ead
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 38 deletions.
104 changes: 66 additions & 38 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5463,42 +5463,72 @@ void LibraryCallKit::create_new_uncommon_trap(CallStaticJavaNode* uncommon_trap_
uncommon_trap_call->set_req(0, top()); // not used anymore, kill it
}

// Common checks for array sorting intrinsics arguments.
// Returns `true` if checks passed.
bool LibraryCallKit::check_array_sort_arguments(Node* elementType, Node* obj, BasicType& bt) {
// check address of the class
if (elementType == nullptr || elementType->is_top()) {
return false; // dead path
}
const TypeInstPtr* elem_klass = gvn().type(elementType)->isa_instptr();
if (elem_klass == nullptr) {
return false; // dead path
}
// java_mirror_type() returns non-null for compile-time Class constants only
ciType* elem_type = elem_klass->java_mirror_type();
if (elem_type == nullptr) {
return false;
}
bt = elem_type->basic_type();
// Disable the intrinsic if the CPU does not support SIMD sort
if (!Matcher::supports_simd_sort(bt)) {
return false;
}
// check address of the array
if (obj == nullptr || obj->is_top()) {
return false; // dead path
}
const TypeAryPtr* obj_t = _gvn.type(obj)->isa_aryptr();
if (obj_t == nullptr || obj_t->elem() == Type::BOTTOM) {
return false; // failed input validation
}
return true;
}

//------------------------------inline_array_partition-----------------------
bool LibraryCallKit::inline_array_partition() {
address stubAddr = StubRoutines::select_array_partition_function();
if (stubAddr == nullptr) {
return false; // Intrinsic's stub is not implemented on this platform
}
assert(callee()->signature()->size() == 9, "arrayPartition has 8 parameters (one long)");

Node* elementType = null_check(argument(0));
// no receiver because it is a static method
Node* elementType = argument(0);
Node* obj = argument(1);
Node* offset = argument(2);
Node* offset = argument(2); // long
Node* fromIndex = argument(4);
Node* toIndex = argument(5);
Node* indexPivot1 = argument(6);
Node* indexPivot2 = argument(7);
// PartitionOperation: argument(8) is ignored

Node* pivotIndices = nullptr;
BasicType bt = T_ILLEGAL;

if (!check_array_sort_arguments(elementType, obj, bt)) {
return false;
}
null_check(obj);
// If obj is dead, only null-path is taken.
if (stopped()) {
return true;
}
// Set the original stack and the reexecute bit for the interpreter to reexecute
// the bytecode that invokes DualPivotQuicksort.partition() if deoptimization happens.
{ PreserveReexecuteState preexecs(this);
jvms()->set_should_reexecute(true);

const TypeInstPtr* elem_klass = gvn().type(elementType)->isa_instptr();
ciType* elem_type = elem_klass->const_oop()->as_instance()->java_mirror_type();
BasicType bt = elem_type->basic_type();
// Disable the intrinsic if the CPU does not support SIMD sort
if (!Matcher::supports_simd_sort(bt)) {
return false;
}
address stubAddr = nullptr;
stubAddr = StubRoutines::select_array_partition_function();
// stub not loaded
if (stubAddr == nullptr) {
return false;
}
// get the address of the array
const TypeAryPtr* obj_t = _gvn.type(obj)->isa_aryptr();
if (obj_t == nullptr || obj_t->elem() == Type::BOTTOM ) {
return false; // failed input validation
}
Node* obj_adr = make_unsafe_address(obj, offset);

// create the pivotIndices array of type int and size = 2
Expand Down Expand Up @@ -5531,31 +5561,29 @@ bool LibraryCallKit::inline_array_partition() {

//------------------------------inline_array_sort-----------------------
bool LibraryCallKit::inline_array_sort() {
address stubAddr = StubRoutines::select_arraysort_function();
if (stubAddr == nullptr) {
return false; // Intrinsic's stub is not implemented on this platform
}
assert(callee()->signature()->size() == 7, "arraySort has 6 parameters (one long)");

Node* elementType = null_check(argument(0));
// no receiver because it is a static method
Node* elementType = argument(0);
Node* obj = argument(1);
Node* offset = argument(2);
Node* offset = argument(2); // long
Node* fromIndex = argument(4);
Node* toIndex = argument(5);
// SortOperation: argument(6) is ignored

const TypeInstPtr* elem_klass = gvn().type(elementType)->isa_instptr();
ciType* elem_type = elem_klass->const_oop()->as_instance()->java_mirror_type();
BasicType bt = elem_type->basic_type();
// Disable the intrinsic if the CPU does not support SIMD sort
if (!Matcher::supports_simd_sort(bt)) {
return false;
}
address stubAddr = nullptr;
stubAddr = StubRoutines::select_arraysort_function();
//stub not loaded
if (stubAddr == nullptr) {
BasicType bt = T_ILLEGAL;

if (!check_array_sort_arguments(elementType, obj, bt)) {
return false;
}

// get address of the array
const TypeAryPtr* obj_t = _gvn.type(obj)->isa_aryptr();
if (obj_t == nullptr || obj_t->elem() == Type::BOTTOM ) {
return false; // failed input validation
null_check(obj);
// If obj is dead, only null-path is taken.
if (stopped()) {
return true;
}
Node* obj_adr = make_unsafe_address(obj, offset);

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/opto/library_call.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ class LibraryCallKit : public GraphKit {
JVMState* arraycopy_restore_alloc_state(AllocateArrayNode* alloc, int& saved_reexecute_sp);
void arraycopy_move_allocation_here(AllocateArrayNode* alloc, Node* dest, JVMState* saved_jvms_before_guards, int saved_reexecute_sp,
uint new_idx);
bool check_array_sort_arguments(Node* elementType, Node* obj, BasicType& bt);
bool inline_array_sort();
bool inline_array_partition();
typedef enum { LS_get_add, LS_get_set, LS_cmp_swap, LS_cmp_swap_weak, LS_cmp_exchange } LoadStoreKind;
Expand Down

3 comments on commit b6d0ead

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@JesperIRL
Copy link
Member

Choose a reason for hiding this comment

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

/tag jdk-23+30

@openjdk
Copy link

@openjdk openjdk bot commented on b6d0ead Jul 4, 2024

Choose a reason for hiding this comment

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

@JesperIRL The tag jdk-23+30 was successfully created.

Please sign in to comment.