From 0e94f8ce51f83627a931d18078f3ec3b5a6a45d3 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Mon, 10 Jun 2024 13:58:08 +0200 Subject: [PATCH 1/3] Variant 3: Handle everything, including reads by compilers --- src/hotspot/share/c1/c1_GraphBuilder.cpp | 12 +- src/hotspot/share/c1/c1_IR.cpp | 1 + src/hotspot/share/c1/c1_IR.hpp | 3 + src/hotspot/share/c1/c1_LIRGenerator.cpp | 11 ++ src/hotspot/share/ci/ciInstance.cpp | 6 +- .../share/classfile/classFileParser.cpp | 2 +- src/hotspot/share/opto/parse.hpp | 10 +- src/hotspot/share/opto/parse1.cpp | 29 +-- src/hotspot/share/opto/parse3.cpp | 37 +++- src/hotspot/share/runtime/globals.hpp | 3 + .../irTests/stable/StablePrimArrayTest.java | 169 +++++++++++++++++ .../irTests/stable/StablePrimFinalTest.java | 99 ++++++++++ .../irTests/stable/StablePrimPlainTest.java | 113 +++++++++++ .../stable/StablePrimVolatileTest.java | 113 +++++++++++ .../c2/irTests/stable/StableRefArrayTest.java | 178 ++++++++++++++++++ .../c2/irTests/stable/StableRefFinalTest.java | 96 ++++++++++ .../c2/irTests/stable/StableRefPlainTest.java | 117 ++++++++++++ .../irTests/stable/StableRefVolatileTest.java | 117 ++++++++++++ 18 files changed, 1074 insertions(+), 42 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java create mode 100644 test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java diff --git a/src/hotspot/share/c1/c1_GraphBuilder.cpp b/src/hotspot/share/c1/c1_GraphBuilder.cpp index 46d74db6d6a50..7c0f45dc2e3b4 100644 --- a/src/hotspot/share/c1/c1_GraphBuilder.cpp +++ b/src/hotspot/share/c1/c1_GraphBuilder.cpp @@ -1563,7 +1563,7 @@ void GraphBuilder::method_return(Value x, bool ignore_return) { // The conditions for a memory barrier are described in Parse::do_exits(). bool need_mem_bar = false; if (method()->name() == ciSymbols::object_initializer_name() && - (scope()->wrote_final() || + (scope()->wrote_final() || scope()->wrote_stable() || (AlwaysSafeConstructors && scope()->wrote_fields()) || (support_IRIW_for_not_multiple_copy_atomic_cpu && scope()->wrote_volatile()))) { need_mem_bar = true; @@ -1741,15 +1741,17 @@ void GraphBuilder::access_field(Bytecodes::Code code) { } } - if (field->is_final() && (code == Bytecodes::_putfield)) { - scope()->set_wrote_final(); - } - if (code == Bytecodes::_putfield) { scope()->set_wrote_fields(); if (field->is_volatile()) { scope()->set_wrote_volatile(); } + if (field->is_final()) { + scope()->set_wrote_final(); + } + if (field->is_stable()) { + scope()->set_wrote_stable(); + } } const int offset = !needs_patching ? field->offset_in_bytes() : -1; diff --git a/src/hotspot/share/c1/c1_IR.cpp b/src/hotspot/share/c1/c1_IR.cpp index e375d16aafb18..b3faa54cc69b8 100644 --- a/src/hotspot/share/c1/c1_IR.cpp +++ b/src/hotspot/share/c1/c1_IR.cpp @@ -146,6 +146,7 @@ IRScope::IRScope(Compilation* compilation, IRScope* caller, int caller_bci, ciMe _wrote_final = false; _wrote_fields = false; _wrote_volatile = false; + _wrote_stable = false; _start = nullptr; if (osr_bci != -1) { diff --git a/src/hotspot/share/c1/c1_IR.hpp b/src/hotspot/share/c1/c1_IR.hpp index 3035643708a81..e2582c77b3921 100644 --- a/src/hotspot/share/c1/c1_IR.hpp +++ b/src/hotspot/share/c1/c1_IR.hpp @@ -149,6 +149,7 @@ class IRScope: public CompilationResourceObj { bool _wrote_final; // has written final field bool _wrote_fields; // has written fields bool _wrote_volatile; // has written volatile field + bool _wrote_stable; // has written @Stable field BlockBegin* _start; // the start block, successsors are method entries ResourceBitMap _requires_phi_function; // bit is set if phi functions at loop headers are necessary for a local variable @@ -187,6 +188,8 @@ class IRScope: public CompilationResourceObj { bool wrote_fields () const { return _wrote_fields; } void set_wrote_volatile() { _wrote_volatile = true; } bool wrote_volatile () const { return _wrote_volatile; } + void set_wrote_stable() { _wrote_stable = true; } + bool wrote_stable() const { return _wrote_stable; } }; diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp b/src/hotspot/share/c1/c1_LIRGenerator.cpp index cd08413565808..178852614ffcb 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.cpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp @@ -1677,7 +1677,18 @@ void LIRGenerator::do_StoreField(StoreField* x) { DecoratorSet decorators = IN_HEAP; if (is_volatile) { + // Volatile access, full SC. decorators |= MO_SEQ_CST; + } else if (x->field()->is_stable() && !x->field()->is_final() && + is_reference_type(field_type)) { + // For reference @Stable fields, make sure we publish the contents + // safely. We need this to make sure compilers see a proper value when + // constant folding the access. Final @Stable fields are already + // handled in constructors. + decorators |= MO_RELEASE; + } else { + // Everything else is unordered. + decorators |= MO_UNORDERED; } if (needs_patching) { decorators |= C1_NEEDS_PATCHING; diff --git a/src/hotspot/share/ci/ciInstance.cpp b/src/hotspot/share/ci/ciInstance.cpp index 6ede59dd72309..d9716b4409ff3 100644 --- a/src/hotspot/share/ci/ciInstance.cpp +++ b/src/hotspot/share/ci/ciInstance.cpp @@ -78,7 +78,11 @@ ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) { case T_LONG: value = ciConstant(obj->long_field(offset)); break; case T_OBJECT: // fall through case T_ARRAY: { - oop o = obj->obj_field(offset); + // In case we are reading the constant off a reference @Stable field, + // we need to match the releasing store with this acquire. There is no easy + // way to know this from the offset, but it should not hurt to ask this for + // all reference fields. + oop o = obj->obj_field_acquire(offset); // A field will be "constant" if it is known always to be // a non-null reference to an instance of a particular class, diff --git a/src/hotspot/share/classfile/classFileParser.cpp b/src/hotspot/share/classfile/classFileParser.cpp index 535b67887fca1..953c44f07c80e 100644 --- a/src/hotspot/share/classfile/classFileParser.cpp +++ b/src/hotspot/share/classfile/classFileParser.cpp @@ -1970,7 +1970,7 @@ AnnotationCollector::annotation_index(const ClassLoaderData* loader_data, } case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Stable_signature): { if (_location != _in_field) break; // only allow for fields - if (!privileged) break; // only allow in privileged code + if (RestrictStable && !privileged) break; // only allow in privileged code return _field_Stable; } case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Contended_signature): { diff --git a/src/hotspot/share/opto/parse.hpp b/src/hotspot/share/opto/parse.hpp index a55c9cb0cb12e..a2690aa6704f0 100644 --- a/src/hotspot/share/opto/parse.hpp +++ b/src/hotspot/share/opto/parse.hpp @@ -358,7 +358,7 @@ class Parse : public GraphKit { bool _wrote_volatile; // Did we write a volatile field? bool _wrote_stable; // Did we write a @Stable field? bool _wrote_fields; // Did we write any field? - Node* _alloc_with_final; // An allocation node with final field + Node* _alloc_with_final_or_stable; // An allocation node with final or @Stable field // Variables which track Java semantics during bytecode parsing: @@ -403,10 +403,10 @@ class Parse : public GraphKit { void set_wrote_stable(bool z) { _wrote_stable = z; } bool wrote_fields() const { return _wrote_fields; } void set_wrote_fields(bool z) { _wrote_fields = z; } - Node* alloc_with_final() const { return _alloc_with_final; } - void set_alloc_with_final(Node* n) { - assert((_alloc_with_final == nullptr) || (_alloc_with_final == n), "different init objects?"); - _alloc_with_final = n; + Node* alloc_with_final_or_stable() const { return _alloc_with_final_or_stable; } + void set_alloc_with_final_or_stable(Node* n) { + assert((_alloc_with_final_or_stable == nullptr) || (_alloc_with_final_or_stable == n), "different init objects?"); + _alloc_with_final_or_stable = n; } Block* block() const { return _block; } diff --git a/src/hotspot/share/opto/parse1.cpp b/src/hotspot/share/opto/parse1.cpp index 3989020451eb5..378e5a6abc446 100644 --- a/src/hotspot/share/opto/parse1.cpp +++ b/src/hotspot/share/opto/parse1.cpp @@ -412,7 +412,7 @@ Parse::Parse(JVMState* caller, ciMethod* parse_method, float expected_uses) _wrote_volatile = false; _wrote_stable = false; _wrote_fields = false; - _alloc_with_final = nullptr; + _alloc_with_final_or_stable = nullptr; _block = nullptr; _first_return = true; _replaced_nodes_for_exceptions = false; @@ -984,8 +984,8 @@ void Parse::do_exits() { // Figure out if we need to emit the trailing barrier. The barrier is only // needed in the constructors, and only in three cases: // - // 1. The constructor wrote a final. The effects of all initializations - // must be committed to memory before any code after the constructor + // 1. The constructor wrote a final or a @Stable field. All these + // initializations must be ordered before any code after the constructor // publishes the reference to the newly constructed object. Rather // than wait for the publication, we simply block the writes here. // Rather than put a barrier on only those writes which are required @@ -1010,34 +1010,23 @@ void Parse::do_exits() { // exceptional returns, since they cannot publish normally. // if (method()->is_initializer() && - (wrote_final() || + (wrote_final() || wrote_stable() || (AlwaysSafeConstructors && wrote_fields()) || (support_IRIW_for_not_multiple_copy_atomic_cpu && wrote_volatile()))) { + Node* recorded_alloc = alloc_with_final_or_stable(); _exits.insert_mem_bar(UseStoreStoreForCtor ? Op_MemBarStoreStore : Op_MemBarRelease, - alloc_with_final()); + recorded_alloc); // If Memory barrier is created for final fields write // and allocation node does not escape the initialize method, // then barrier introduced by allocation node can be removed. - if (DoEscapeAnalysis && alloc_with_final()) { - AllocateNode* alloc = AllocateNode::Ideal_allocation(alloc_with_final()); + if (DoEscapeAnalysis && (recorded_alloc != nullptr)) { + AllocateNode* alloc = AllocateNode::Ideal_allocation(recorded_alloc); alloc->compute_MemBar_redundancy(method()); } if (PrintOpto && (Verbose || WizardMode)) { method()->print_name(); - tty->print_cr(" writes finals and needs a memory barrier"); - } - } - - // Any method can write a @Stable field; insert memory barriers - // after those also. Can't bind predecessor allocation node (if any) - // with barrier because allocation doesn't always dominate - // MemBarRelease. - if (wrote_stable()) { - _exits.insert_mem_bar(Op_MemBarRelease); - if (PrintOpto && (Verbose || WizardMode)) { - method()->print_name(); - tty->print_cr(" writes @Stable and needs a memory barrier"); + tty->print_cr(" writes finals/@Stable and needs a memory barrier"); } } diff --git a/src/hotspot/share/opto/parse3.cpp b/src/hotspot/share/opto/parse3.cpp index a2bf653b17651..10b2cc87df73f 100644 --- a/src/hotspot/share/opto/parse3.cpp +++ b/src/hotspot/share/opto/parse3.cpp @@ -209,7 +209,21 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) { Node* val = type2size[bt] == 1 ? pop() : pop_pair(); DecoratorSet decorators = IN_HEAP; - decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED; + + if (is_vol) { + // Volatile access, full SC. + decorators |= MO_SEQ_CST; + } else if (field->is_stable() && !field->is_final() && + is_reference_type(field->layout_type())) { + // For reference @Stable fields, make sure we publish the contents + // safely. We need this to make sure compilers see a proper value when + // constant folding the access. Final @Stable fields are already + // handled in constructors. + decorators |= MO_RELEASE; + } else { + // Everything else is unordered. + decorators |= MO_UNORDERED; + } bool is_obj = is_reference_type(bt); @@ -236,22 +250,25 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) { set_wrote_fields(true); // If the field is final, the rules of Java say we are in or . - // Note the presence of writes to final non-static fields, so that we + // If the field is @Stable, we can be in any method, but we only care about + // constructors at this point. + // + // Note the presence of writes to final/@Stable non-static fields, so that we // can insert a memory barrier later on to keep the writes from floating // out of the constructor. - // Any method can write a @Stable field; insert memory barriers after those also. - if (field->is_final()) { - set_wrote_final(true); + if (field->is_final() || field->is_stable()) { + if (field->is_final()) { + set_wrote_final(true); + } + if (field->is_stable()) { + set_wrote_stable(true); + } if (AllocateNode::Ideal_allocation(obj) != nullptr) { // Preserve allocation ptr to create precedent edge to it in membar // generated on exit from constructor. - // Can't bind stable with its allocation, only record allocation for final field. - set_alloc_with_final(obj); + set_alloc_with_final_or_stable(obj); } } - if (field->is_stable()) { - set_wrote_stable(true); - } } } diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index cbd161d86eb48..4b0f2b81d3408 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -1987,6 +1987,9 @@ const int ObjectAlignmentInBytes = 8; \ product(bool, StressSecondarySupers, false, DIAGNOSTIC, \ "Use a terrible hash function in order to generate many collisions.") \ + \ + develop(bool, RestrictStable, true, \ + "Restrict @Stable to trusted classes") \ // end of RUNTIME_FLAGS diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java new file mode 100644 index 0000000000000..d89f9235caa5d --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java @@ -0,0 +1,169 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StablePrimArrayTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StablePrimArrayTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static final int[] EMPTY_INTEGER = new int[] { 0 }; + static final int[] FULL_INTEGER = new int[] { 42 }; + + static class Carrier { + @Stable + int[] field; + + @ForceInline + public Carrier(int initLevel) { + switch (initLevel) { + case 0: + // Do nothing. + break; + case 1: + field = EMPTY_INTEGER; + break; + case 2: + field = FULL_INTEGER; + break; + default: + throw new IllegalStateException("Unknown level"); + } + } + + @ForceInline + public void initEmpty() { + field = EMPTY_INTEGER; + } + + @ForceInline + public void initFull() { + field = FULL_INTEGER; + } + + } + + static final Carrier BLANK_CARRIER = new Carrier(0); + static final Carrier INIT_EMPTY_CARRIER = new Carrier(1); + static final Carrier INIT_FULL_CARRIER = new Carrier(2); + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for plain fields. + int[] is = BLANK_CARRIER.field; + if (is != null) { + return is[0]; + } + return 0; + } + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testPartialFold() { + // Access should not be folded. + // No barriers expected for plain fields. + int[] is = INIT_EMPTY_CARRIER.field; + if (is != null) { + return is[0]; + } + return 0; + } + + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + int[] is = INIT_FULL_CARRIER.field; + if (is != null) { + return is[0]; + } + return 0; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Only the header barrier. + return new Carrier(0); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorEmptyInit() { + // Only the header barrier. + return new Carrier(1); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorFullInit() { + // Only the header barrier. + return new Carrier(2); + } + + @Test + @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + static void testMethodEmptyInit() { + // Reference inits have release membars. + INIT_EMPTY_CARRIER.initEmpty(); + } + + @Test + @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + static void testMethodFullInit() { + // Reference inits have release membars. + INIT_FULL_CARRIER.initFull(); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java new file mode 100644 index 0000000000000..62f687fed46eb --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java @@ -0,0 +1,99 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StablePrimFinalTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StablePrimFinalTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static class Carrier { + @Stable + final int field; + + @ForceInline + public Carrier(boolean init) { + field = init ? 42 : 0; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, "1" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for final fields. + return BLANK_CARRIER.field; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + return INIT_CARRIER.field; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Single header+final barrier. + return new Carrier(false); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorFullInit() { + // Single header+final barrier. + return new Carrier(true); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java new file mode 100644 index 0000000000000..74eeed18e7680 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java @@ -0,0 +1,113 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StablePrimPlainTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StablePrimPlainTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static class Carrier { + @Stable + int field; + + @ForceInline + public Carrier(boolean init) { + if (init) { + field = 42; + } + } + + @ForceInline + public void init() { + field = 42; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, "1" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for plain fields. + return BLANK_CARRIER.field; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + return INIT_CARRIER.field; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Only the header barrier. + return new Carrier(false); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorFullInit() { + // Only the header barrier. + return new Carrier(true); + } + + @Test + @IR(failOn = { IRNode.MEMBAR }) + static void testMethodInit() { + // Primitive inits have no membars. + INIT_CARRIER.init(); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java new file mode 100644 index 0000000000000..bcddd595da232 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java @@ -0,0 +1,113 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StablePrimVolatileTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StablePrimVolatileTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static class Carrier { + @Stable + volatile int field; + + @ForceInline + public Carrier(boolean init) { + if (init) { + field = 42; + } + } + + @ForceInline + public void init() { + field = 42; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, "1" }) + @IR(counts = { IRNode.MEMBAR, ">0" }) + static int testNoFold() { + // Access should not be folded. + // Barriers expected for volatile fields. + return BLANK_CARRIER.field; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + return INIT_CARRIER.field; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Expect only the header barrier. + return new Carrier(false); + } + + @Test + @IR(counts = { IRNode.MEMBAR, ">0" }) + static Carrier testConstructorFullInit() { + // Volatile barriers expected. + return new Carrier(true); + } + + @Test + @IR(counts = { IRNode.MEMBAR, ">0" }) + static void testMethodInit() { + // Volatile barriers expected. + INIT_CARRIER.init(); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java new file mode 100644 index 0000000000000..c0e81ee947476 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java @@ -0,0 +1,178 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StableRefArrayTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StableRefArrayTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static final Integer[] EMPTY_INTEGER = new Integer[] { null }; + static final Integer[] FULL_INTEGER = new Integer[] { 42 }; + + static class Carrier { + @Stable + Integer[] field; + + @ForceInline + public Carrier(int initLevel) { + switch (initLevel) { + case 0: + // Do nothing. + break; + case 1: + field = EMPTY_INTEGER; + break; + case 2: + field = FULL_INTEGER; + break; + default: + throw new IllegalStateException("Unknown level"); + } + } + + @ForceInline + public void initEmpty() { + field = EMPTY_INTEGER; + } + + @ForceInline + public void initFull() { + field = FULL_INTEGER; + } + + } + + static final Carrier BLANK_CARRIER = new Carrier(0); + static final Carrier INIT_EMPTY_CARRIER = new Carrier(1); + static final Carrier INIT_FULL_CARRIER = new Carrier(2); + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for plain fields. + Integer[] is = BLANK_CARRIER.field; + if (is != null) { + Integer i = is[0]; + if (i != null) { + return i; + } + } + return 0; + } + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testPartialFold() { + // Access should not be folded. + // No barriers expected for plain fields. + Integer[] is = INIT_EMPTY_CARRIER.field; + if (is != null) { + Integer i = is[0]; + if (i != null) { + return i; + } + } + return 0; + } + + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + Integer[] is = INIT_FULL_CARRIER.field; + if (is != null) { + Integer i = is[0]; + if (i != null) { + return i; + } + } + return 0; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Only the header barrier. + return new Carrier(0); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorEmptyInit() { + // Only the header barrier. + return new Carrier(1); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorFullInit() { + // Only the header barrier. + return new Carrier(2); + } + + @Test + @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + static void testMethodEmptyInit() { + // Reference inits have release membars. + INIT_EMPTY_CARRIER.initEmpty(); + } + + @Test + @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + static void testMethodFullInit() { + // Reference inits have release membars. + INIT_FULL_CARRIER.initFull(); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java new file mode 100644 index 0000000000000..6b813db90098a --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java @@ -0,0 +1,96 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StableRefFinalTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StableRefFinalTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static final Integer INTEGER = 42; + + static class Carrier { + @Stable + final Integer field; + + @ForceInline + public Carrier(boolean init) { + field = init ? INTEGER : null; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for plain fields. + Integer i = BLANK_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + Integer i = INIT_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorInit() { + // Only the header+final barrier. + return new Carrier(true); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java new file mode 100644 index 0000000000000..145765f1334b5 --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java @@ -0,0 +1,117 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StableRefPlainTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StableRefPlainTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static final Integer INTEGER = 42; + + static class Carrier { + @Stable + Integer field; + + @ForceInline + public Carrier(boolean init) { + if (init) { + field = INTEGER; + } + } + + @ForceInline + public void init() { + field = INTEGER; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(failOn = { IRNode.MEMBAR }) + static int testNoFold() { + // Access should not be folded. + // No barriers expected for plain fields. + Integer i = BLANK_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + Integer i = INIT_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Only the header barrier. + return new Carrier(false); + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorFullInit() { + // Only the header barrier. + return new Carrier(true); + } + + @Test + @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + static void testMethodInit() { + // Reference inits have release membars. + INIT_CARRIER.init(); + } + +} diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java new file mode 100644 index 0000000000000..fa4487eac574b --- /dev/null +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java @@ -0,0 +1,117 @@ +/* + * Copyright Amazon.com Inc. 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. + */ + +/* + * @test + * @bug 8333791 + * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" + * @requires vm.gc.Parallel + * @requires vm.compiler2.enabled + * @requires vm.debug + * @summary Check stable field folding and barriers + * @modules java.base/jdk.internal.vm.annotation + * @library /test/lib / + * @run driver compiler.c2.irTests.stable.StableRefVolatileTest + */ + +package compiler.c2.irTests.stable; + +import compiler.lib.ir_framework.*; +import jdk.test.lib.Asserts; + +import jdk.internal.vm.annotation.Stable; + +public class StableRefVolatileTest { + + public static void main(String[] args) { + TestFramework.runWithFlags( + "-XX:+UnlockExperimentalVMOptions", + "-XX:CompileThreshold=100", + "-XX:-TieredCompilation", + "-XX:+UseParallelGC", + "-XX:-RestrictStable" + ); + } + + static final Integer INTEGER = 42; + + static class Carrier { + @Stable + volatile Integer field; + + @ForceInline + public Carrier(boolean init) { + if (init) { + field = INTEGER; + } + } + + @ForceInline + public void init() { + field = INTEGER; + } + } + + static final Carrier BLANK_CARRIER = new Carrier(false); + static final Carrier INIT_CARRIER = new Carrier(true); + + @Test + @IR(counts = { IRNode.LOAD, ">0" }) + @IR(counts = { IRNode.MEMBAR, ">0" }) + static int testNoFold() { + // Access should not be folded. + // Barriers are expected for volatile field. + Integer i = BLANK_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(failOn = { IRNode.LOAD, IRNode.MEMBAR }) + static int testFold() { + // Access should be completely folded. + Integer i = INIT_CARRIER.field; + return i != null ? i : 0; + } + + @Test + @IR(counts = { IRNode.MEMBAR_STORESTORE, "1" }) + static Carrier testConstructorBlankInit() { + // Only the header barrier. + return new Carrier(false); + } + + @Test + @IR(counts = { IRNode.MEMBAR, ">0" }) + static Carrier testConstructorFullInit() { + // Volatile writes, expect more barriers. + return new Carrier(true); + } + + @Test + @IR(counts = { IRNode.MEMBAR, ">0" }) + static void testMethodInit() { + // Barriers are expected for volatile fields. + INIT_CARRIER.init(); + } + +} From 3892d37fa3b195cf35877781e57b796fd51ec5c0 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 26 Jun 2024 21:35:22 +0200 Subject: [PATCH 2/3] Variant 2: Only final-field like semantics for stable inits --- src/hotspot/share/c1/c1_LIRGenerator.cpp | 11 ----------- src/hotspot/share/ci/ciInstance.cpp | 6 +----- src/hotspot/share/opto/parse3.cpp | 16 +--------------- .../c2/irTests/stable/StablePrimArrayTest.java | 8 ++++---- .../c2/irTests/stable/StableRefArrayTest.java | 8 ++++---- .../c2/irTests/stable/StableRefPlainTest.java | 4 ++-- 6 files changed, 12 insertions(+), 41 deletions(-) diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp b/src/hotspot/share/c1/c1_LIRGenerator.cpp index 178852614ffcb..cd08413565808 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.cpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp @@ -1677,18 +1677,7 @@ void LIRGenerator::do_StoreField(StoreField* x) { DecoratorSet decorators = IN_HEAP; if (is_volatile) { - // Volatile access, full SC. decorators |= MO_SEQ_CST; - } else if (x->field()->is_stable() && !x->field()->is_final() && - is_reference_type(field_type)) { - // For reference @Stable fields, make sure we publish the contents - // safely. We need this to make sure compilers see a proper value when - // constant folding the access. Final @Stable fields are already - // handled in constructors. - decorators |= MO_RELEASE; - } else { - // Everything else is unordered. - decorators |= MO_UNORDERED; } if (needs_patching) { decorators |= C1_NEEDS_PATCHING; diff --git a/src/hotspot/share/ci/ciInstance.cpp b/src/hotspot/share/ci/ciInstance.cpp index d9716b4409ff3..6ede59dd72309 100644 --- a/src/hotspot/share/ci/ciInstance.cpp +++ b/src/hotspot/share/ci/ciInstance.cpp @@ -78,11 +78,7 @@ ciConstant ciInstance::field_value_impl(BasicType field_btype, int offset) { case T_LONG: value = ciConstant(obj->long_field(offset)); break; case T_OBJECT: // fall through case T_ARRAY: { - // In case we are reading the constant off a reference @Stable field, - // we need to match the releasing store with this acquire. There is no easy - // way to know this from the offset, but it should not hurt to ask this for - // all reference fields. - oop o = obj->obj_field_acquire(offset); + oop o = obj->obj_field(offset); // A field will be "constant" if it is known always to be // a non-null reference to an instance of a particular class, diff --git a/src/hotspot/share/opto/parse3.cpp b/src/hotspot/share/opto/parse3.cpp index 10b2cc87df73f..a9fad4e3633e6 100644 --- a/src/hotspot/share/opto/parse3.cpp +++ b/src/hotspot/share/opto/parse3.cpp @@ -209,21 +209,7 @@ void Parse::do_put_xxx(Node* obj, ciField* field, bool is_field) { Node* val = type2size[bt] == 1 ? pop() : pop_pair(); DecoratorSet decorators = IN_HEAP; - - if (is_vol) { - // Volatile access, full SC. - decorators |= MO_SEQ_CST; - } else if (field->is_stable() && !field->is_final() && - is_reference_type(field->layout_type())) { - // For reference @Stable fields, make sure we publish the contents - // safely. We need this to make sure compilers see a proper value when - // constant folding the access. Final @Stable fields are already - // handled in constructors. - decorators |= MO_RELEASE; - } else { - // Everything else is unordered. - decorators |= MO_UNORDERED; - } + decorators |= is_vol ? MO_SEQ_CST : MO_UNORDERED; bool is_obj = is_reference_type(bt); diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java index d89f9235caa5d..0bda71caadc83 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java @@ -153,16 +153,16 @@ static Carrier testConstructorFullInit() { } @Test - @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + @IR(failOn = { IRNode.MEMBAR }) static void testMethodEmptyInit() { - // Reference inits have release membars. + // Reference inits do not have membars. INIT_EMPTY_CARRIER.initEmpty(); } @Test - @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + @IR(failOn = { IRNode.MEMBAR }) static void testMethodFullInit() { - // Reference inits have release membars. + // Reference inits do not have membars. INIT_FULL_CARRIER.initFull(); } diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java index c0e81ee947476..7cf49c243c4dc 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java @@ -162,16 +162,16 @@ static Carrier testConstructorFullInit() { } @Test - @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + @IR(failOn = { IRNode.MEMBAR }) static void testMethodEmptyInit() { - // Reference inits have release membars. + // Reference inits do not have membars. INIT_EMPTY_CARRIER.initEmpty(); } @Test - @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + @IR(failOn = { IRNode.MEMBAR }) static void testMethodFullInit() { - // Reference inits have release membars. + // Reference inits do not have membars. INIT_FULL_CARRIER.initFull(); } diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java index 145765f1334b5..0e5df47c5b644 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java @@ -108,9 +108,9 @@ static Carrier testConstructorFullInit() { } @Test - @IR(counts = { IRNode.MEMBAR_RELEASE, "1" }) + @IR(failOn = { IRNode.MEMBAR }) static void testMethodInit() { - // Reference inits have release membars. + // Reference inits do not have membars. INIT_CARRIER.init(); } From 52c838163feea75a0b09e490cbccef831f4d4435 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Mon, 12 Aug 2024 12:00:52 +0200 Subject: [PATCH 3/3] Use TestFramework bootclasspath instead of develop option --- src/hotspot/share/classfile/classFileParser.cpp | 2 +- src/hotspot/share/runtime/globals.hpp | 3 --- .../compiler/c2/irTests/stable/StablePrimArrayTest.java | 9 +++++---- .../compiler/c2/irTests/stable/StablePrimFinalTest.java | 9 +++++---- .../compiler/c2/irTests/stable/StablePrimPlainTest.java | 9 +++++---- .../c2/irTests/stable/StablePrimVolatileTest.java | 9 +++++---- .../compiler/c2/irTests/stable/StableRefArrayTest.java | 9 +++++---- .../compiler/c2/irTests/stable/StableRefFinalTest.java | 9 +++++---- .../compiler/c2/irTests/stable/StableRefPlainTest.java | 9 +++++---- .../c2/irTests/stable/StableRefVolatileTest.java | 9 +++++---- 10 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/hotspot/share/classfile/classFileParser.cpp b/src/hotspot/share/classfile/classFileParser.cpp index 2b83064a7d43b..407078d64fc57 100644 --- a/src/hotspot/share/classfile/classFileParser.cpp +++ b/src/hotspot/share/classfile/classFileParser.cpp @@ -1950,7 +1950,7 @@ AnnotationCollector::annotation_index(const ClassLoaderData* loader_data, } case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Stable_signature): { if (_location != _in_field) break; // only allow for fields - if (RestrictStable && !privileged) break; // only allow in privileged code + if (!privileged) break; // only allow in privileged code return _field_Stable; } case VM_SYMBOL_ENUM_NAME(jdk_internal_vm_annotation_Contended_signature): { diff --git a/src/hotspot/share/runtime/globals.hpp b/src/hotspot/share/runtime/globals.hpp index 0e5456db5d058..61efc0b93764d 100644 --- a/src/hotspot/share/runtime/globals.hpp +++ b/src/hotspot/share/runtime/globals.hpp @@ -1986,9 +1986,6 @@ const int ObjectAlignmentInBytes = 8; \ product(bool, StressSecondarySupers, false, DIAGNOSTIC, \ "Use a terrible hash function in order to generate many collisions.") \ - \ - develop(bool, RestrictStable, true, \ - "Restrict @Stable to trusted classes") \ // end of RUNTIME_FLAGS diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java index 0bda71caadc83..24733700f81de 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimArrayTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StablePrimArrayTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static final int[] EMPTY_INTEGER = new int[] { 0 }; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java index 62f687fed46eb..355fadf6cc1e1 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimFinalTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StablePrimFinalTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static class Carrier { diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java index 74eeed18e7680..38cc8bfad4e5e 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimPlainTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StablePrimPlainTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static class Carrier { diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java index bcddd595da232..159c2f7321d55 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StablePrimVolatileTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StablePrimVolatileTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static class Carrier { diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java index 7cf49c243c4dc..027bd2dce3046 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefArrayTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StableRefArrayTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static final Integer[] EMPTY_INTEGER = new Integer[] { null }; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java index 6b813db90098a..405c86a5fc9e2 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefFinalTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StableRefFinalTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static final Integer INTEGER = 42; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java index 0e5df47c5b644..bd5be32459d9c 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefPlainTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StableRefPlainTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static final Integer INTEGER = 42; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java index fa4487eac574b..7d7d7fcad7795 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/stable/StableRefVolatileTest.java @@ -27,7 +27,6 @@ * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" | os.arch=="amd64" * @requires vm.gc.Parallel * @requires vm.compiler2.enabled - * @requires vm.debug * @summary Check stable field folding and barriers * @modules java.base/jdk.internal.vm.annotation * @library /test/lib / @@ -44,13 +43,15 @@ public class StableRefVolatileTest { public static void main(String[] args) { - TestFramework.runWithFlags( + TestFramework tf = new TestFramework(); + tf.addTestClassesToBootClassPath(); + tf.addFlags( "-XX:+UnlockExperimentalVMOptions", "-XX:CompileThreshold=100", "-XX:-TieredCompilation", - "-XX:+UseParallelGC", - "-XX:-RestrictStable" + "-XX:+UseParallelGC" ); + tf.start(); } static final Integer INTEGER = 42;