From da74fbd920cdcd16f6097fcb8488a061f4753be5 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 13 Jan 2025 13:45:19 +0000 Subject: [PATCH] 8347006: LoadRangeNode floats above array guard in arraycopy intrinsic Reviewed-by: chagedorn Backport-of: 82e2a791225a289ba32360bf415274c4b48b9e00 --- src/hotspot/share/opto/library_call.cpp | 28 ++++++++++++------- src/hotspot/share/opto/library_call.hpp | 20 ++++++------- src/hotspot/share/opto/type.cpp | 24 ++++++++-------- src/hotspot/share/opto/type.hpp | 23 +++++++-------- .../arraycopy/TestArrayCopyNoInit.java | 7 +++-- 5 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 1072a5d6a24..fcf6b920884 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -4262,7 +4262,7 @@ bool LibraryCallKit::inline_native_subtype_check() { //---------------------generate_array_guard_common------------------------ Node* LibraryCallKit::generate_array_guard_common(Node* kls, RegionNode* region, - bool obj_array, bool not_array) { + bool obj_array, bool not_array, Node** obj) { if (stopped()) { return nullptr; @@ -4304,7 +4304,14 @@ Node* LibraryCallKit::generate_array_guard_common(Node* kls, RegionNode* region, // invert the test if we are looking for a non-array if (not_array) btest = BoolTest(btest).negate(); Node* bol = _gvn.transform(new BoolNode(cmp, btest)); - return generate_fair_guard(bol, region); + Node* ctrl = generate_fair_guard(bol, region); + Node* is_array_ctrl = not_array ? control() : ctrl; + if (obj != nullptr && is_array_ctrl != nullptr && is_array_ctrl != top()) { + // Keep track of the fact that 'obj' is an array to prevent + // array specific accesses from floating above the guard. + *obj = _gvn.transform(new CastPPNode(is_array_ctrl, *obj, TypeAryPtr::BOTTOM)); + } + return ctrl; } @@ -4399,7 +4406,7 @@ bool LibraryCallKit::inline_native_getLength() { if (stopped()) return true; // Deoptimize if it is a non-array. - Node* non_array = generate_non_array_guard(load_object_klass(array), nullptr); + Node* non_array = generate_non_array_guard(load_object_klass(array), nullptr, &array); if (non_array != nullptr) { PreserveJVMState pjvms(this); @@ -5259,12 +5266,13 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) { record_for_igvn(result_reg); Node* obj_klass = load_object_klass(obj); - Node* array_ctl = generate_array_guard(obj_klass, (RegionNode*)nullptr); + Node* array_obj = obj; + Node* array_ctl = generate_array_guard(obj_klass, (RegionNode*)nullptr, &array_obj); if (array_ctl != nullptr) { // It's an array. PreserveJVMState pjvms(this); set_control(array_ctl); - Node* obj_length = load_array_length(obj); + Node* obj_length = load_array_length(array_obj); Node* array_size = nullptr; // Size of the array without object alignment padding. Node* alloc_obj = new_array(obj_klass, obj_length, 0, &array_size, /*deoptimize_on_exception=*/true); @@ -5278,7 +5286,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) { set_control(is_obja); // Generate a direct call to the right arraycopy function(s). // Clones are always tightly coupled. - ArrayCopyNode* ac = ArrayCopyNode::make(this, true, obj, intcon(0), alloc_obj, intcon(0), obj_length, true, false); + ArrayCopyNode* ac = ArrayCopyNode::make(this, true, array_obj, intcon(0), alloc_obj, intcon(0), obj_length, true, false); ac->set_clone_oop_array(); Node* n = _gvn.transform(ac); assert(n == ac, "cannot disappear"); @@ -5299,7 +5307,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) { // the object.) if (!stopped()) { - copy_to_clone(obj, alloc_obj, array_size, true); + copy_to_clone(array_obj, alloc_obj, array_size, true); // Present the results of the copy. result_reg->init_req(_array_path, control()); @@ -5920,8 +5928,8 @@ bool LibraryCallKit::inline_arraycopy() { record_for_igvn(slow_region); // (1) src and dest are arrays. - generate_non_array_guard(load_object_klass(src), slow_region); - generate_non_array_guard(load_object_klass(dest), slow_region); + generate_non_array_guard(load_object_klass(src), slow_region, &src); + generate_non_array_guard(load_object_klass(dest), slow_region, &dest); // (2) src and dest arrays must have elements of the same BasicType // done at macro expansion or at Ideal transformation time @@ -8537,7 +8545,7 @@ bool LibraryCallKit::inline_getObjectSize() { PhiNode* result_val = new PhiNode(result_reg, TypeLong::LONG); record_for_igvn(result_reg); - Node* array_ctl = generate_array_guard(klass_node, nullptr); + Node* array_ctl = generate_array_guard(klass_node, nullptr, &obj); if (array_ctl != nullptr) { // Array case: size is round(header + element_size*arraylength). // Since arraylength is different for every array instance, we have to diff --git a/src/hotspot/share/opto/library_call.hpp b/src/hotspot/share/opto/library_call.hpp index c5437e3bf73..25a853a6fef 100644 --- a/src/hotspot/share/opto/library_call.hpp +++ b/src/hotspot/share/opto/library_call.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -163,20 +163,20 @@ class LibraryCallKit : public GraphKit { RegionNode* region); Node* generate_interface_guard(Node* kls, RegionNode* region); Node* generate_hidden_class_guard(Node* kls, RegionNode* region); - Node* generate_array_guard(Node* kls, RegionNode* region) { - return generate_array_guard_common(kls, region, false, false); + Node* generate_array_guard(Node* kls, RegionNode* region, Node** obj = nullptr) { + return generate_array_guard_common(kls, region, false, false, obj); } - Node* generate_non_array_guard(Node* kls, RegionNode* region) { - return generate_array_guard_common(kls, region, false, true); + Node* generate_non_array_guard(Node* kls, RegionNode* region, Node** obj = nullptr) { + return generate_array_guard_common(kls, region, false, true, obj); } - Node* generate_objArray_guard(Node* kls, RegionNode* region) { - return generate_array_guard_common(kls, region, true, false); + Node* generate_objArray_guard(Node* kls, RegionNode* region, Node** obj = nullptr) { + return generate_array_guard_common(kls, region, true, false, obj); } - Node* generate_non_objArray_guard(Node* kls, RegionNode* region) { - return generate_array_guard_common(kls, region, true, true); + Node* generate_non_objArray_guard(Node* kls, RegionNode* region, Node** obj = nullptr) { + return generate_array_guard_common(kls, region, true, true, obj); } Node* generate_array_guard_common(Node* kls, RegionNode* region, - bool obj_array, bool not_array); + bool obj_array, bool not_array, Node** obj = nullptr); Node* generate_virtual_guard(Node* obj_klass, RegionNode* slow_region); CallJavaNode* generate_method_call(vmIntrinsicID method_id, bool is_virtual, bool is_static, bool res_not_null); CallJavaNode* generate_method_call_static(vmIntrinsicID method_id, bool res_not_null) { diff --git a/src/hotspot/share/opto/type.cpp b/src/hotspot/share/opto/type.cpp index 407a4a20a9b..2424daf81e3 100644 --- a/src/hotspot/share/opto/type.cpp +++ b/src/hotspot/share/opto/type.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 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 @@ -582,6 +582,7 @@ void Type::Initialize_shared(Compile* current) { TypeAryPtr::_array_interfaces = TypeInterfaces::make(&array_interfaces); TypeAryKlassPtr::_array_interfaces = TypeAryPtr::_array_interfaces; + TypeAryPtr::BOTTOM = TypeAryPtr::make(TypePtr::BotPTR, TypeAry::make(Type::BOTTOM, TypeInt::POS), nullptr, false, Type::OffsetBot); TypeAryPtr::RANGE = TypeAryPtr::make( TypePtr::BotPTR, TypeAry::make(Type::BOTTOM,TypeInt::POS), nullptr /* current->env()->Object_klass() */, false, arrayOopDesc::length_offset_in_bytes()); TypeAryPtr::NARROWOOPS = TypeAryPtr::make(TypePtr::BotPTR, TypeAry::make(TypeNarrowOop::BOTTOM, TypeInt::POS), nullptr /*ciArrayKlass::make(o)*/, false, Type::OffsetBot); @@ -4682,16 +4683,17 @@ bool TypeAryKlassPtr::is_meet_subtype_of_helper(const TypeKlassPtr *other, bool //============================================================================= // Convenience common pre-built types. -const TypeAryPtr *TypeAryPtr::RANGE; -const TypeAryPtr *TypeAryPtr::OOPS; -const TypeAryPtr *TypeAryPtr::NARROWOOPS; -const TypeAryPtr *TypeAryPtr::BYTES; -const TypeAryPtr *TypeAryPtr::SHORTS; -const TypeAryPtr *TypeAryPtr::CHARS; -const TypeAryPtr *TypeAryPtr::INTS; -const TypeAryPtr *TypeAryPtr::LONGS; -const TypeAryPtr *TypeAryPtr::FLOATS; -const TypeAryPtr *TypeAryPtr::DOUBLES; +const TypeAryPtr* TypeAryPtr::BOTTOM; +const TypeAryPtr* TypeAryPtr::RANGE; +const TypeAryPtr* TypeAryPtr::OOPS; +const TypeAryPtr* TypeAryPtr::NARROWOOPS; +const TypeAryPtr* TypeAryPtr::BYTES; +const TypeAryPtr* TypeAryPtr::SHORTS; +const TypeAryPtr* TypeAryPtr::CHARS; +const TypeAryPtr* TypeAryPtr::INTS; +const TypeAryPtr* TypeAryPtr::LONGS; +const TypeAryPtr* TypeAryPtr::FLOATS; +const TypeAryPtr* TypeAryPtr::DOUBLES; //------------------------------make------------------------------------------- const TypeAryPtr *TypeAryPtr::make(PTR ptr, const TypeAry *ary, ciKlass* k, bool xk, int offset, diff --git a/src/hotspot/share/opto/type.hpp b/src/hotspot/share/opto/type.hpp index f6b7efcae3b..9bb86fc94e6 100644 --- a/src/hotspot/share/opto/type.hpp +++ b/src/hotspot/share/opto/type.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 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 @@ -1470,16 +1470,17 @@ public: virtual const TypeKlassPtr* as_klass_type(bool try_for_exact = false) const; // Convenience common pre-built types. - static const TypeAryPtr *RANGE; - static const TypeAryPtr *OOPS; - static const TypeAryPtr *NARROWOOPS; - static const TypeAryPtr *BYTES; - static const TypeAryPtr *SHORTS; - static const TypeAryPtr *CHARS; - static const TypeAryPtr *INTS; - static const TypeAryPtr *LONGS; - static const TypeAryPtr *FLOATS; - static const TypeAryPtr *DOUBLES; + static const TypeAryPtr* BOTTOM; + static const TypeAryPtr* RANGE; + static const TypeAryPtr* OOPS; + static const TypeAryPtr* NARROWOOPS; + static const TypeAryPtr* BYTES; + static const TypeAryPtr* SHORTS; + static const TypeAryPtr* CHARS; + static const TypeAryPtr* INTS; + static const TypeAryPtr* LONGS; + static const TypeAryPtr* FLOATS; + static const TypeAryPtr* DOUBLES; // selects one of the above: static const TypeAryPtr *get_array_body_type(BasicType elem) { assert((uint)elem <= T_CONFLICT && _array_body_type[elem] != nullptr, "bad elem type"); diff --git a/test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyNoInit.java b/test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyNoInit.java index 828fb5ace99..c85a0d6ac1c 100644 --- a/test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyNoInit.java +++ b/test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyNoInit.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -23,11 +23,14 @@ /* * @test - * @bug 8064703 + * @bug 8064703 8347006 * @summary Deoptimization between array allocation and arraycopy may result in non initialized array * * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:TypeProfileLevel=020 * compiler.arraycopy.TestArrayCopyNoInit + * @run main/othervm -XX:-BackgroundCompilation -XX:-UseOnStackReplacement -XX:TypeProfileLevel=020 + * -XX:+UnlockExperimentalVMOptions -XX:+UseCompactObjectHeaders -XX:-UseTLAB + * compiler.arraycopy.TestArrayCopyNoInit */ package compiler.arraycopy;