From 63e611cd5d7eb4fc6ea6633ff9123e4bee5f5993 Mon Sep 17 00:00:00 2001 From: Tobias Hartmann Date: Mon, 23 Sep 2024 12:30:30 +0000 Subject: [PATCH] 8335334: Stress mode to randomly execute unstable if traps Reviewed-by: chagedorn, kvn --- src/hotspot/share/opto/c2_globals.hpp | 3 + src/hotspot/share/opto/cfgnode.hpp | 2 +- src/hotspot/share/opto/compile.cpp | 10 +-- src/hotspot/share/opto/ifnode.cpp | 4 +- src/hotspot/share/opto/parse.hpp | 5 ++ src/hotspot/share/opto/parse2.cpp | 77 +++++++++++++++++++ .../compiler/arguments/TestStressOptions.java | 10 ++- .../c2/irTests/TestPrunedExHandler.java | 1 + .../cha/StrengthReduceInterfaceCall.java | 1 + .../MaterializeVirtualObjectTest.java | 1 + .../rangechecks/TestExplicitRangeChecks.java | 1 + .../uncommontrap/TestUnstableIfTrap.java | 2 +- .../runner/BasicBooleanOpTest.java | 1 + .../whitebox/DeoptimizeFramesTest.java | 1 + .../event/compiler/TestDeoptimization.java | 1 + 15 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index 7288533cd33..42de77acca9 100644 --- a/src/hotspot/share/opto/c2_globals.hpp +++ b/src/hotspot/share/opto/c2_globals.hpp @@ -58,6 +58,9 @@ product(bool, StressMacroExpansion, false, DIAGNOSTIC, \ "Randomize macro node expansion order") \ \ + product(bool, StressUnstableIfTraps, false, DIAGNOSTIC, \ + "Randomly take unstable if traps") \ + \ product(uint, StressSeed, 0, DIAGNOSTIC, \ "Seed for randomized stress testing (if unset, a random one is " \ "generated). The seed is recorded in the compilation log, if " \ diff --git a/src/hotspot/share/opto/cfgnode.hpp b/src/hotspot/share/opto/cfgnode.hpp index 5edd31fa716..ac8896705de 100644 --- a/src/hotspot/share/opto/cfgnode.hpp +++ b/src/hotspot/share/opto/cfgnode.hpp @@ -347,7 +347,6 @@ class IfNode : public MultiBranchNode { bool is_null_check(ProjNode* proj, PhaseIterGVN* igvn); bool is_side_effect_free_test(ProjNode* proj, PhaseIterGVN* igvn); void reroute_side_effect_free_unc(ProjNode* proj, ProjNode* dom_proj, PhaseIterGVN* igvn); - ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call) const; bool fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* fail, PhaseIterGVN* igvn); static bool is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* unc); @@ -442,6 +441,7 @@ public: static Node* up_one_dom(Node* curr, bool linear_only = false); bool is_zero_trip_guard() const; Node* dominated_by(Node* prev_dom, PhaseIterGVN* igvn, bool pin_array_access_nodes); + ProjNode* uncommon_trap_proj(CallStaticJavaNode*& call, Deoptimization::DeoptReason reason = Deoptimization::Reason_none) const; // Takes the type of val and filters it through the test represented // by if_proj and returns a more refined type if one is produced. diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 6382608f36a..5abc398cb8c 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -719,6 +719,11 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, method()->ensure_method_data(); } + if (StressLCM || StressGCM || StressIGVN || StressCCP || + StressIncrementalInlining || StressMacroExpansion || StressUnstableIfTraps) { + initialize_stress_seed(directive); + } + Init(/*do_aliasing=*/ true); print_compile_messages(); @@ -843,11 +848,6 @@ Compile::Compile( ciEnv* ci_env, ciMethod* target, int osr_bci, if (failing()) return; NOT_PRODUCT( verify_graph_edges(); ) - if (StressLCM || StressGCM || StressIGVN || StressCCP || - StressIncrementalInlining || StressMacroExpansion) { - initialize_stress_seed(directive); - } - // Now optimize Optimize(); if (failing()) return; diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 8e5cd270213..4313b2cf907 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -840,9 +840,9 @@ bool IfNode::is_dominator_unc(CallStaticJavaNode* dom_unc, CallStaticJavaNode* u } // Return projection that leads to an uncommon trap if any -ProjNode* IfNode::uncommon_trap_proj(CallStaticJavaNode*& call) const { +ProjNode* IfNode::uncommon_trap_proj(CallStaticJavaNode*& call, Deoptimization::DeoptReason reason) const { for (int i = 0; i < 2; i++) { - call = proj_out(i)->is_uncommon_trap_proj(); + call = proj_out(i)->is_uncommon_trap_proj(reason); if (call != nullptr) { return proj_out(i); } diff --git a/src/hotspot/share/opto/parse.hpp b/src/hotspot/share/opto/parse.hpp index a2690aa6704..484c49367cc 100644 --- a/src/hotspot/share/opto/parse.hpp +++ b/src/hotspot/share/opto/parse.hpp @@ -612,6 +612,11 @@ class Parse : public GraphKit { // Use speculative type to optimize CmpP node Node* optimize_cmp_with_klass(Node* c); + // Stress unstable if traps + void stress_trap(IfNode* orig_iff, Node* counter, Node* incr_store); + // Increment counter used by StressUnstableIfTraps + void increment_trap_stress_counter(Node*& counter, Node*& incr_store); + public: #ifndef PRODUCT // Handle PrintOpto, etc. diff --git a/src/hotspot/share/opto/parse2.cpp b/src/hotspot/share/opto/parse2.cpp index cbd9323c3a9..6b7cb4eaa99 100644 --- a/src/hotspot/share/opto/parse2.cpp +++ b/src/hotspot/share/opto/parse2.cpp @@ -1371,10 +1371,27 @@ inline int Parse::repush_if_args() { return bc_depth; } +// Used by StressUnstableIfTraps +static volatile int _trap_stress_counter = 0; + +void Parse::increment_trap_stress_counter(Node*& counter, Node*& incr_store) { + Node* counter_addr = makecon(TypeRawPtr::make((address)&_trap_stress_counter)); + counter = make_load(control(), counter_addr, TypeInt::INT, T_INT, Compile::AliasIdxRaw, MemNode::unordered); + counter = _gvn.transform(new AddINode(counter, intcon(1))); + incr_store = store_to_memory(control(), counter_addr, counter, T_INT, Compile::AliasIdxRaw, MemNode::unordered); +} + //----------------------------------do_ifnull---------------------------------- void Parse::do_ifnull(BoolTest::mask btest, Node *c) { int target_bci = iter().get_dest(); + Node* counter = nullptr; + Node* incr_store = nullptr; + bool do_stress_trap = StressUnstableIfTraps && ((C->random() % 2) == 0); + if (do_stress_trap) { + increment_trap_stress_counter(counter, incr_store); + } + Block* branch_block = successor_for_bci(target_bci); Block* next_block = successor_for_bci(iter().next_bci()); @@ -1439,6 +1456,10 @@ void Parse::do_ifnull(BoolTest::mask btest, Node *c) { } else { // Path is live. adjust_map_after_if(BoolTest(btest).negate(), c, 1.0-prob, next_block); } + + if (do_stress_trap) { + stress_trap(iff, counter, incr_store); + } } //------------------------------------do_if------------------------------------ @@ -1468,6 +1489,13 @@ void Parse::do_if(BoolTest::mask btest, Node* c) { return; } + Node* counter = nullptr; + Node* incr_store = nullptr; + bool do_stress_trap = StressUnstableIfTraps && ((C->random() % 2) == 0); + if (do_stress_trap) { + increment_trap_stress_counter(counter, incr_store); + } + // Sanity check the probability value assert(0.0f < prob && prob < 1.0f,"Bad probability in Parser"); @@ -1550,9 +1578,58 @@ void Parse::do_if(BoolTest::mask btest, Node* c) { } else { adjust_map_after_if(untaken_btest, c, untaken_prob, next_block); } + + if (do_stress_trap) { + stress_trap(iff, counter, incr_store); + } +} + +// Force unstable if traps to be taken randomly to trigger intermittent bugs such as incorrect debug information. +// Add another if before the unstable if that checks a "random" condition at runtime (a simple shared counter) and +// then either takes the trap or executes the original, unstable if. +void Parse::stress_trap(IfNode* orig_iff, Node* counter, Node* incr_store) { + // Search for an unstable if trap + CallStaticJavaNode* trap = nullptr; + assert(orig_iff->Opcode() == Op_If && orig_iff->outcnt() == 2, "malformed if"); + ProjNode* trap_proj = orig_iff->uncommon_trap_proj(trap, Deoptimization::Reason_unstable_if); + if (trap == nullptr || !trap->jvms()->should_reexecute()) { + // No suitable trap found. Remove unused counter load and increment. + C->gvn_replace_by(incr_store, incr_store->in(MemNode::Memory)); + return; + } + + // Remove trap from optimization list since we add another path to the trap. + bool success = C->remove_unstable_if_trap(trap, true); + assert(success, "Trap already modified"); + + // Add a check before the original if that will trap with a certain frequency and execute the original if otherwise + int freq_log = (C->random() % 31) + 1; // Random logarithmic frequency in [1, 31] + Node* mask = intcon(right_n_bits(freq_log)); + counter = _gvn.transform(new AndINode(counter, mask)); + Node* cmp = _gvn.transform(new CmpINode(counter, intcon(0))); + Node* bol = _gvn.transform(new BoolNode(cmp, BoolTest::mask::eq)); + IfNode* iff = _gvn.transform(new IfNode(orig_iff->in(0), bol, orig_iff->_prob, orig_iff->_fcnt))->as_If(); + Node* if_true = _gvn.transform(new IfTrueNode(iff)); + Node* if_false = _gvn.transform(new IfFalseNode(iff)); + assert(!if_true->is_top() && !if_false->is_top(), "trap always / never taken"); + + // Trap + assert(trap_proj->outcnt() == 1, "some other nodes are dependent on the trap projection"); + + Node* trap_region = new RegionNode(3); + trap_region->set_req(1, trap_proj); + trap_region->set_req(2, if_true); + trap->set_req(0, _gvn.transform(trap_region)); + + // Don't trap, execute original if + orig_iff->set_req(0, if_false); } bool Parse::path_is_suitable_for_uncommon_trap(float prob) const { + // Randomly skip emitting an uncommon trap + if (StressUnstableIfTraps && ((C->random() % 2) == 0)) { + return false; + } // Don't want to speculate on uncommon traps when running with -Xcomp if (!UseInterpreter) { return false; diff --git a/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java b/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java index 544f84c9b27..ccf4822e676 100644 --- a/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java +++ b/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java @@ -24,7 +24,7 @@ /* * @test * @key stress randomness - * @bug 8252219 8256535 8317349 + * @bug 8252219 8256535 8317349 8319879 8335334 * @requires vm.compiler2.enabled * @summary Tests that different combinations of stress options and * -XX:StressSeed=N are accepted. @@ -48,6 +48,14 @@ * compiler.arguments.TestStressOptions * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressMacroExpansion -XX:StressSeed=42 * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressIncrementalInlining + * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressIncrementalInlining -XX:StressSeed=42 + * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressUnstableIfTraps + * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressUnstableIfTraps -XX:StressSeed=42 + * compiler.arguments.TestStressOptions */ package compiler.arguments; diff --git a/test/hotspot/jtreg/compiler/c2/irTests/TestPrunedExHandler.java b/test/hotspot/jtreg/compiler/c2/irTests/TestPrunedExHandler.java index 9b91375acb7..b5a27f3f714 100644 --- a/test/hotspot/jtreg/compiler/c2/irTests/TestPrunedExHandler.java +++ b/test/hotspot/jtreg/compiler/c2/irTests/TestPrunedExHandler.java @@ -29,6 +29,7 @@ import compiler.lib.ir_framework.*; * @test * @bug 8267532 * @summary check that uncommon trap is generated for unhandled catch block + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @library /test/lib / * @requires vm.opt.DeoptimizeALot != true * @run driver compiler.c2.irTests.TestPrunedExHandler diff --git a/test/hotspot/jtreg/compiler/cha/StrengthReduceInterfaceCall.java b/test/hotspot/jtreg/compiler/cha/StrengthReduceInterfaceCall.java index 068da4100bd..87ce9a313a8 100644 --- a/test/hotspot/jtreg/compiler/cha/StrengthReduceInterfaceCall.java +++ b/test/hotspot/jtreg/compiler/cha/StrengthReduceInterfaceCall.java @@ -24,6 +24,7 @@ /* * @test * @requires !vm.graal.enabled + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @modules java.base/jdk.internal.org.objectweb.asm * java.base/jdk.internal.misc * java.base/jdk.internal.vm.annotation diff --git a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java index 5855440d0b2..1dcef0c96cd 100644 --- a/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java +++ b/test/hotspot/jtreg/compiler/jvmci/compilerToVM/MaterializeVirtualObjectTest.java @@ -27,6 +27,7 @@ * * @requires vm.jvmci & vm.compMode == "Xmixed" * @requires vm.opt.final.EliminateAllocations == true + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * * @comment no "-Xcomp -XX:-TieredCompilation" combination allowed until JDK-8140018 is resolved * @requires vm.opt.TieredCompilation == null | vm.opt.TieredCompilation == true diff --git a/test/hotspot/jtreg/compiler/rangechecks/TestExplicitRangeChecks.java b/test/hotspot/jtreg/compiler/rangechecks/TestExplicitRangeChecks.java index ba045814fc2..83bfd63c629 100644 --- a/test/hotspot/jtreg/compiler/rangechecks/TestExplicitRangeChecks.java +++ b/test/hotspot/jtreg/compiler/rangechecks/TestExplicitRangeChecks.java @@ -25,6 +25,7 @@ * @test * @bug 8073480 * @summary explicit range checks should be recognized by C2 + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @library /test/lib / * @modules java.base/jdk.internal.misc:+open * @build jdk.test.whitebox.WhiteBox diff --git a/test/hotspot/jtreg/compiler/uncommontrap/TestUnstableIfTrap.java b/test/hotspot/jtreg/compiler/uncommontrap/TestUnstableIfTrap.java index 5fee59c7510..626f4c9b1f6 100644 --- a/test/hotspot/jtreg/compiler/uncommontrap/TestUnstableIfTrap.java +++ b/test/hotspot/jtreg/compiler/uncommontrap/TestUnstableIfTrap.java @@ -24,7 +24,7 @@ /* * @test * @bug 8030976 8059226 - * @requires !vm.graal.enabled + * @requires !vm.graal.enabled & (vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps) * @library /test/lib / * @modules java.base/jdk.internal.org.objectweb.asm * java.base/jdk.internal.misc diff --git a/test/hotspot/jtreg/compiler/vectorization/runner/BasicBooleanOpTest.java b/test/hotspot/jtreg/compiler/vectorization/runner/BasicBooleanOpTest.java index bb6185177e7..18e7721e0d6 100644 --- a/test/hotspot/jtreg/compiler/vectorization/runner/BasicBooleanOpTest.java +++ b/test/hotspot/jtreg/compiler/vectorization/runner/BasicBooleanOpTest.java @@ -25,6 +25,7 @@ /* * @test * @summary Vectorization test on basic boolean operations + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @library /test/lib / * * @build jdk.test.whitebox.WhiteBox diff --git a/test/hotspot/jtreg/compiler/whitebox/DeoptimizeFramesTest.java b/test/hotspot/jtreg/compiler/whitebox/DeoptimizeFramesTest.java index 1d9c8169d04..91601479881 100644 --- a/test/hotspot/jtreg/compiler/whitebox/DeoptimizeFramesTest.java +++ b/test/hotspot/jtreg/compiler/whitebox/DeoptimizeFramesTest.java @@ -25,6 +25,7 @@ * @test DeoptimizeFramesTest * @bug 8028595 * @summary testing of WB::deoptimizeFrames() + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @library /test/lib / * @modules java.base/jdk.internal.misc * java.management diff --git a/test/jdk/jdk/jfr/event/compiler/TestDeoptimization.java b/test/jdk/jdk/jfr/event/compiler/TestDeoptimization.java index 32c634fc59a..daf562a765f 100644 --- a/test/jdk/jdk/jfr/event/compiler/TestDeoptimization.java +++ b/test/jdk/jdk/jfr/event/compiler/TestDeoptimization.java @@ -53,6 +53,7 @@ class Dummy { * @requires vm.hasJFR * @requires vm.compMode != "Xint" * @requires vm.flavor == "server" & (vm.opt.TieredStopAtLevel == 4 | vm.opt.TieredStopAtLevel == null) + * @requires vm.opt.StressUnstableIfTraps == null | !vm.opt.StressUnstableIfTraps * @library /test/lib * @build jdk.test.whitebox.WhiteBox * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox