diff --git a/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp b/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp index 41bd15ab000..90fae8c3588 100644 --- a/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp +++ b/src/hotspot/share/gc/shared/c2/barrierSetC2.cpp @@ -675,15 +675,8 @@ void BarrierSetC2::clone(GraphKit* kit, Node* src_base, Node* dst_base, Node* si Node* payload_size = size; Node* offset = kit->MakeConX(base_off); payload_size = kit->gvn().transform(new SubXNode(payload_size, offset)); - if (is_array) { - // Ensure the array payload size is rounded up to the next BytesPerLong - // multiple when converting to double-words. This is necessary because array - // size does not include object alignment padding, so it might not be a - // multiple of BytesPerLong for sub-long element types. - payload_size = kit->gvn().transform(new AddXNode(payload_size, kit->MakeConX(BytesPerLong - 1))); - } payload_size = kit->gvn().transform(new URShiftXNode(payload_size, kit->intcon(LogBytesPerLong))); - ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, offset, dst_base, offset, payload_size, true, false); + ArrayCopyNode* ac = ArrayCopyNode::make(kit, false, src_base, offset, dst_base, offset, payload_size, true, false); if (is_array) { ac->set_clone_array(); } else { diff --git a/src/hotspot/share/opto/graphKit.cpp b/src/hotspot/share/opto/graphKit.cpp index e3fe25c925b..d9ea82c03c1 100644 --- a/src/hotspot/share/opto/graphKit.cpp +++ b/src/hotspot/share/opto/graphKit.cpp @@ -3728,10 +3728,7 @@ Node* GraphKit::new_instance(Node* klass_node, //-------------------------------new_array------------------------------------- // helper for both newarray and anewarray // The 'length' parameter is (obviously) the length of the array. -// The optional arguments are for specialized use by intrinsics: -// - If 'return_size_val', report the non-padded array size (sum of header size -// and array body) to the caller. -// - deoptimize_on_exception controls how Java exceptions are handled (rethrow vs deoptimize) +// See comments on new_instance for the meaning of the other arguments. Node* GraphKit::new_array(Node* klass_node, // array klass (maybe variable) Node* length, // number of array elements int nargs, // number of arguments to push back for uncommon trap @@ -3782,21 +3779,25 @@ Node* GraphKit::new_array(Node* klass_node, // array klass (maybe variable) // The rounding mask is strength-reduced, if possible. int round_mask = MinObjAlignmentInBytes - 1; Node* header_size = nullptr; + int header_size_min = arrayOopDesc::base_offset_in_bytes(T_BYTE); // (T_BYTE has the weakest alignment and size restrictions...) if (layout_is_con) { int hsize = Klass::layout_helper_header_size(layout_con); int eshift = Klass::layout_helper_log2_element_size(layout_con); + BasicType etype = Klass::layout_helper_element_type(layout_con); if ((round_mask & ~right_n_bits(eshift)) == 0) round_mask = 0; // strength-reduce it if it goes away completely assert((hsize & right_n_bits(eshift)) == 0, "hsize is pre-rounded"); - int header_size_min = arrayOopDesc::base_offset_in_bytes(T_BYTE); assert(header_size_min <= hsize, "generic minimum is smallest"); - header_size = intcon(hsize); + header_size_min = hsize; + header_size = intcon(hsize + round_mask); } else { Node* hss = intcon(Klass::_lh_header_size_shift); Node* hsm = intcon(Klass::_lh_header_size_mask); - header_size = _gvn.transform(new URShiftINode(layout_val, hss)); - header_size = _gvn.transform(new AndINode(header_size, hsm)); + Node* hsize = _gvn.transform( new URShiftINode(layout_val, hss) ); + hsize = _gvn.transform( new AndINode(hsize, hsm) ); + Node* mask = intcon(round_mask); + header_size = _gvn.transform( new AddINode(hsize, mask) ); } Node* elem_shift = nullptr; @@ -3848,30 +3849,25 @@ Node* GraphKit::new_array(Node* klass_node, // array klass (maybe variable) } #endif - // Combine header size and body size for the array copy part, then align (if - // necessary) for the allocation part. This computation cannot overflow, - // because it is used only in two places, one where the length is sharply - // limited, and the other after a successful allocation. + // Combine header size (plus rounding) and body size. Then round down. + // This computation cannot overflow, because it is used only in two + // places, one where the length is sharply limited, and the other + // after a successful allocation. Node* abody = lengthx; - if (elem_shift != nullptr) { - abody = _gvn.transform(new LShiftXNode(lengthx, elem_shift)); + if (elem_shift != nullptr) + abody = _gvn.transform( new LShiftXNode(lengthx, elem_shift) ); + Node* size = _gvn.transform( new AddXNode(headerx, abody) ); + if (round_mask != 0) { + Node* mask = MakeConX(~round_mask); + size = _gvn.transform( new AndXNode(size, mask) ); } - Node* non_rounded_size = _gvn.transform(new AddXNode(headerx, abody)); + // else if round_mask == 0, the size computation is self-rounding if (return_size_val != nullptr) { // This is the size - (*return_size_val) = non_rounded_size; + (*return_size_val) = size; } - Node* size = non_rounded_size; - if (round_mask != 0) { - Node* mask1 = MakeConX(round_mask); - size = _gvn.transform(new AddXNode(size, mask1)); - Node* mask2 = MakeConX(~round_mask); - size = _gvn.transform(new AndXNode(size, mask2)); - } - // else if round_mask == 0, the size computation is self-rounding - // Now generate allocation code // The entire memory state is needed for slow path of the allocation diff --git a/src/hotspot/share/opto/library_call.cpp b/src/hotspot/share/opto/library_call.cpp index 587c3d84f79..dec7f30d070 100644 --- a/src/hotspot/share/opto/library_call.cpp +++ b/src/hotspot/share/opto/library_call.cpp @@ -5021,8 +5021,8 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) { PreserveJVMState pjvms(this); set_control(array_ctl); Node* obj_length = load_array_length(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); + Node* obj_size = nullptr; + Node* alloc_obj = new_array(obj_klass, obj_length, 0, &obj_size, /*deoptimize_on_exception=*/true); BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2(); if (bs->array_copy_requires_gc_barriers(true, T_OBJECT, true, false, BarrierSetC2::Parsing)) { @@ -5055,7 +5055,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(obj, alloc_obj, obj_size, true); // Present the results of the copy. result_reg->init_req(_array_path, control()); @@ -5095,7 +5095,7 @@ bool LibraryCallKit::inline_native_clone(bool is_virtual) { if (!stopped()) { // It's an instance, and it passed the slow-path tests. PreserveJVMState pjvms(this); - Node* obj_size = nullptr; // Total object size, including object alignment padding. + Node* obj_size = nullptr; // Need to deoptimize on exception from allocation since Object.clone intrinsic // is reexecuted if deoptimization occurs and there could be problems when merging // exception state between multiple Object.clone versions (reexecute=true vs reexecute=false). diff --git a/test/hotspot/jtreg/compiler/gcbarriers/TestArrayCopyWithLargeObjectAlignment.java b/test/hotspot/jtreg/compiler/gcbarriers/TestArrayCopyWithLargeObjectAlignment.java deleted file mode 100644 index dd2d485fb76..00000000000 --- a/test/hotspot/jtreg/compiler/gcbarriers/TestArrayCopyWithLargeObjectAlignment.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright (c) 2023, 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.gcbarriers; - -import java.util.Arrays; - -/** - * @test - * @bug 8312749 - * @summary Test that, when using a larger object alignment, ZGC arraycopy - * barriers are only applied to actual OOPs, and not to object - * alignment padding words. - * @requires vm.gc.ZGenerational - * @run main/othervm -Xbatch -XX:-TieredCompilation - * -XX:CompileOnly=compiler.gcbarriers.TestArrayCopyWithLargeObjectAlignment::* - * -XX:ObjectAlignmentInBytes=16 - * -XX:+UseZGC -XX:+ZGenerational - * compiler.gcbarriers.TestArrayCopyWithLargeObjectAlignment - */ - -public class TestArrayCopyWithLargeObjectAlignment { - - static Object[] doCopyOf(Object[] array) { - return Arrays.copyOf(array, array.length); - } - - static Object[] doClone(Object[] array) { - return array.clone(); - } - - public static void main(String[] args) { - for (int i = 0; i < 10_000; i++) { - // This test allocates an array 'a', copies it into a new array 'b' - // using Arrays.copyOf, and clones 'b' into yet another array. For - // ObjectAlignmentInBytes=16, the intrinsic implementation of - // Arrays.copyOf leaves the object alignment padding word "b[1]" - // untouched, preserving the badHeapWordVal value '0xbaadbabe'. The - // test checks that this padding word is not processed as a valid - // OOP by the ZGC arraycopy stub underlying the intrinsic - // implementation of Object.clone. Allocating b using the intrinsic - // implementation of Arrays.copyOf is key to reproducing the issue - // because, unlike regular (fast or slow) array allocation, - // Arrays.copyOf does not zero-clear the padding word. - Object[] a = {new Object()}; - Object[] b = doCopyOf(a); - doClone(b); - } - } -} diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index 164c54066fa..31546524178 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -478,7 +478,6 @@ java/lang/invoke/LFCaching/LFMultiThreadCachingTest.java 8151492 generic- java/lang/invoke/LFCaching/LFGarbageCollectedTest.java 8078602 generic-all java/lang/invoke/lambda/LambdaFileEncodingSerialization.java 8249079 linux-x64 java/lang/invoke/RicochetTest.java 8251969 generic-all -java/lang/template/StringTemplateTest.java 8315029 generic-all ############################################################################