Skip to content

Commit a06f3cd

Browse files
committed
8373248: C2: CastPP should not change the type of the oop
Reviewed-by: bmaillard, dfenacci, rcastanedalo, mchevalier
1 parent e82f871 commit a06f3cd

5 files changed

Lines changed: 58 additions & 12 deletions

File tree

src/hotspot/share/opto/castnode.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,43 @@ Node* CastLLNode::Ideal(PhaseGVN* phase, bool can_reshape) {
413413
return nullptr;
414414
}
415415

416+
// CastPPNodes are removed before matching, while alias classes are needed in global code motion.
417+
// As a result, it is not valid for a CastPPNode to change the oop such that the derived pointers
418+
// lie in different alias classes with and without the node. For example, a CastPPNode c may not
419+
// cast an Object to a Bottom[], because later removal of c would affect the alias class of c's
420+
// array length field (c + arrayOopDesc::length_offset_in_bytes()).
421+
//
422+
// This function verifies that a CastPPNode on an oop does not violate the aforementioned property.
423+
//
424+
// TODO 8382147: Currently, this verification only applies during the construction of a CastPPNode,
425+
// we may want to apply the same verification during IGVN transformations, as well as final graph
426+
// reshaping.
427+
void CastPPNode::verify_type(const Type* in_type, const Type* out_type) {
428+
#ifdef ASSERT
429+
out_type = out_type->join(in_type);
430+
if (in_type->empty() || out_type->empty()) {
431+
return;
432+
}
433+
if (in_type == TypePtr::NULL_PTR || out_type == TypePtr::NULL_PTR) {
434+
return;
435+
}
436+
if (!in_type->isa_oopptr() && !out_type->isa_oopptr()) {
437+
return;
438+
}
439+
440+
assert(in_type->isa_oopptr() && out_type->isa_oopptr(), "must be both oops or both non-oops");
441+
if (in_type->isa_aryptr() && out_type->isa_aryptr()) {
442+
const Type* e1 = in_type->is_aryptr()->elem();
443+
const Type* e2 = out_type->is_aryptr()->elem();
444+
assert(e1->basic_type() == e2->basic_type(), "must both be arrays of the same primitive type or both be oops arrays");
445+
return;
446+
}
447+
448+
assert(in_type->isa_instptr() && out_type->isa_instptr(), "must be both array oops or both non-array oops");
449+
assert(in_type->is_instptr()->instance_klass() == out_type->is_instptr()->instance_klass(), "must not cast to a different type");
450+
#endif // ASSERT
451+
}
452+
416453
//------------------------------Value------------------------------------------
417454
// Take 'join' of input and cast-up type, unless working with an Interface
418455
const Type* CheckCastPPNode::Value(PhaseGVN* phase) const {
@@ -440,6 +477,11 @@ const Type* CheckCastPPNode::Value(PhaseGVN* phase) const {
440477
return result;
441478
}
442479

480+
Node* CheckCastPPNode::pin_node_under_control_impl() const {
481+
assert(_dependency.is_floating(), "already pinned");
482+
return new CheckCastPPNode(in(0), in(1), bottom_type(), _dependency.with_pinned_dependency(), _extra_types);
483+
}
484+
443485
//=============================================================================
444486
//------------------------------Value------------------------------------------
445487
const Type* CastX2PNode::Value(PhaseGVN* phase) const {

src/hotspot/share/opto/castnode.hpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,14 +303,18 @@ class CastVVNode: public ConstraintCastNode {
303303

304304
//------------------------------CastPPNode-------------------------------------
305305
// cast pointer to pointer (different type)
306-
class CastPPNode: public ConstraintCastNode {
307-
public:
308-
CastPPNode (Node* ctrl, Node* n, const Type* t, const DependencyType& dependency = DependencyType::FloatingNarrowing, const TypeTuple* types = nullptr)
306+
class CastPPNode : public ConstraintCastNode {
307+
public:
308+
CastPPNode(Node* ctrl, Node* n, const Type* t, const DependencyType& dependency = DependencyType::FloatingNarrowing, const TypeTuple* types = nullptr)
309309
: ConstraintCastNode(ctrl, n, t, dependency, types) {
310310
init_class_id(Class_CastPP);
311+
verify_type(n->bottom_type(), t);
311312
}
312313
virtual int Opcode() const;
313314
virtual uint ideal_reg() const { return Op_RegP; }
315+
316+
private:
317+
static void verify_type(const Type* in_type, const Type* out_type);
314318
};
315319

316320
//------------------------------CheckCastPPNode--------------------------------
@@ -329,6 +333,7 @@ class CheckCastPPNode: public ConstraintCastNode {
329333

330334
private:
331335
virtual bool depends_only_on_test_impl() const { return !type()->isa_rawptr() && ConstraintCastNode::depends_only_on_test_impl(); }
336+
virtual Node* pin_node_under_control_impl() const;
332337
};
333338

334339

src/hotspot/share/opto/library_call.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4311,7 +4311,7 @@ Node* LibraryCallKit::generate_array_guard_common(Node* kls, RegionNode* region,
43114311
if (obj != nullptr && is_array_ctrl != nullptr && is_array_ctrl != top()) {
43124312
// Keep track of the fact that 'obj' is an array to prevent
43134313
// array specific accesses from floating above the guard.
4314-
*obj = _gvn.transform(new CastPPNode(is_array_ctrl, *obj, TypeAryPtr::BOTTOM));
4314+
*obj = _gvn.transform(new CheckCastPPNode(is_array_ctrl, *obj, TypeAryPtr::BOTTOM));
43154315
}
43164316
return ctrl;
43174317
}

src/hotspot/share/opto/node.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,7 @@ class Node {
11861186
return nullptr;
11871187
}
11881188
assert(!res->depends_only_on_test(), "the result must not depends_only_on_test");
1189+
assert(Opcode() == res->Opcode(), "pinning must result in the same kind of node %s - %s", Name(), res->Name());
11891190
return res;
11901191
}
11911192

src/hotspot/share/opto/vector.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2020, 2026, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -455,14 +455,12 @@ void PhaseVector::expand_vunbox_node(VectorUnboxNode* vec_unbox) {
455455
gvn.record_for_igvn(local_mem);
456456
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
457457
C2OptAccess access(gvn, ctrl, local_mem, decorators, T_OBJECT, obj, addr);
458-
const Type* type = TypeOopPtr::make_from_klass(field->type()->as_klass());
459-
vec_field_ld = bs->load_at(access, type);
460-
}
461458

462-
// For proper aliasing, attach concrete payload type.
463-
ciKlass* payload_klass = ciTypeArrayKlass::make(bt);
464-
const Type* payload_type = TypeAryPtr::make_from_klass(payload_klass)->cast_to_ptr_type(TypePtr::NotNull);
465-
vec_field_ld = gvn.transform(new CastPPNode(nullptr, vec_field_ld, payload_type));
459+
// For proper aliasing, attach concrete payload type.
460+
ciKlass* payload_klass = ciTypeArrayKlass::make(bt);
461+
const Type* payload_type = TypeAryPtr::make_from_klass(payload_klass)->cast_to_ptr_type(TypePtr::NotNull);
462+
vec_field_ld = bs->load_at(access, payload_type);
463+
}
466464

467465
Node* adr = kit.array_element_address(vec_field_ld, gvn.intcon(0), bt);
468466
const TypePtr* adr_type = adr->bottom_type()->is_ptr();

0 commit comments

Comments
 (0)