From eac8f5d2c99e1bcc526da0f6a05af76e815c2db9 Mon Sep 17 00:00:00 2001 From: Saranya Natarajan Date: Wed, 2 Jul 2025 08:38:31 +0000 Subject: [PATCH] 8325478: Restructure the macro expansion compiler phase to not include macro elimination Reviewed-by: kvn, dlunden --- src/hotspot/share/opto/c2_globals.hpp | 3 ++ src/hotspot/share/opto/compile.cpp | 18 ++++++++-- src/hotspot/share/opto/macro.cpp | 36 ++++++++++++------- src/hotspot/share/opto/macro.hpp | 1 + src/hotspot/share/opto/phasetype.hpp | 2 ++ src/utils/IdealGraphVisualizer/README.md | 2 +- .../compiler/arguments/TestStressOptions.java | 8 +++-- .../debug/TestGenerateStressSeed.java | 5 +-- .../jtreg/compiler/debug/TestStress.java | 12 +++++-- .../lib/ir_framework/CompilePhase.java | 2 ++ .../src/sun/hotspot/tools/ctw/CtwRunner.java | 3 +- 11 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/hotspot/share/opto/c2_globals.hpp b/src/hotspot/share/opto/c2_globals.hpp index ca76114af31..5bfb9045e1c 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, StressMacroElimination, false, DIAGNOSTIC, \ + "Randomize macro node elimination order") \ + \ product(bool, StressUnstableIfTraps, false, DIAGNOSTIC, \ "Randomly take unstable if traps") \ \ diff --git a/src/hotspot/share/opto/compile.cpp b/src/hotspot/share/opto/compile.cpp index 4c5f382ceee..e524249a6cf 100644 --- a/src/hotspot/share/opto/compile.cpp +++ b/src/hotspot/share/opto/compile.cpp @@ -736,8 +736,9 @@ Compile::Compile(ciEnv* ci_env, ciMethod* target, int osr_bci, } if (StressLCM || StressGCM || StressIGVN || StressCCP || - StressIncrementalInlining || StressMacroExpansion || StressUnstableIfTraps || StressBailout || - StressLoopPeeling) { + StressIncrementalInlining || StressMacroExpansion || + StressMacroElimination || StressUnstableIfTraps || + StressBailout || StressLoopPeeling) { initialize_stress_seed(directive); } @@ -2421,6 +2422,7 @@ void Compile::Optimize() { PhaseMacroExpand mexp(igvn); mexp.eliminate_macro_nodes(); if (failing()) return; + print_method(PHASE_AFTER_MACRO_ELIMINATION, 2); igvn.set_delay_transform(false); igvn.optimize(); @@ -2520,6 +2522,18 @@ void Compile::Optimize() { TracePhase tp(_t_macroExpand); print_method(PHASE_BEFORE_MACRO_EXPANSION, 3); PhaseMacroExpand mex(igvn); + // Do not allow new macro nodes once we start to eliminate and expand + C->reset_allow_macro_nodes(); + // Last attempt to eliminate macro nodes before expand + mex.eliminate_macro_nodes(); + if (failing()) { + return; + } + mex.eliminate_opaque_looplimit_macro_nodes(); + if (failing()) { + return; + } + print_method(PHASE_AFTER_MACRO_ELIMINATION, 2); if (mex.expand_macro_nodes()) { assert(failing(), "must bail out w/ explicit message"); return; diff --git a/src/hotspot/share/opto/macro.cpp b/src/hotspot/share/opto/macro.cpp index 8f21ee13e79..d7914f04c1f 100644 --- a/src/hotspot/share/opto/macro.cpp +++ b/src/hotspot/share/opto/macro.cpp @@ -2365,6 +2365,10 @@ void PhaseMacroExpand::refine_strip_mined_loop_macro_nodes() { void PhaseMacroExpand::eliminate_macro_nodes() { if (C->macro_count() == 0) return; + + if (StressMacroElimination) { + C->shuffle_macro_nodes(); + } NOT_PRODUCT(int membar_before = count_MemBar(C);) // Before elimination may re-mark (change to Nested or NonEscObj) @@ -2404,6 +2408,9 @@ void PhaseMacroExpand::eliminate_macro_nodes() { } assert(success == (C->macro_count() < old_macro_count), "elimination reduces macro count"); progress = progress || success; + if (success) { + C->print_method(PHASE_AFTER_MACRO_ELIMINATION_STEP, 5, n); + } } } // Next, attempt to eliminate allocations @@ -2452,6 +2459,9 @@ void PhaseMacroExpand::eliminate_macro_nodes() { } assert(success == (C->macro_count() < old_macro_count), "elimination reduces macro count"); progress = progress || success; + if (success) { + C->print_method(PHASE_AFTER_MACRO_ELIMINATION_STEP, 5, n); + } } } #ifndef PRODUCT @@ -2462,19 +2472,11 @@ void PhaseMacroExpand::eliminate_macro_nodes() { #endif } -//------------------------------expand_macro_nodes---------------------- -// Returns true if a failure occurred. -bool PhaseMacroExpand::expand_macro_nodes() { - refine_strip_mined_loop_macro_nodes(); - // Do not allow new macro nodes once we started to expand - C->reset_allow_macro_nodes(); - if (StressMacroExpansion) { - C->shuffle_macro_nodes(); +void PhaseMacroExpand::eliminate_opaque_looplimit_macro_nodes() { + if (C->macro_count() == 0) { + return; } - // Last attempt to eliminate macro nodes. - eliminate_macro_nodes(); - if (C->failing()) return true; - + refine_strip_mined_loop_macro_nodes(); // Eliminate Opaque and LoopLimit nodes. Do it after all loop optimizations. bool progress = true; while (progress) { @@ -2536,10 +2538,18 @@ bool PhaseMacroExpand::expand_macro_nodes() { assert(!success || (C->macro_count() == (old_macro_count - 1)), "elimination must have deleted one node from macro list"); progress = progress || success; if (success) { - C->print_method(PHASE_AFTER_MACRO_EXPANSION_STEP, 5, n); + C->print_method(PHASE_AFTER_MACRO_ELIMINATION_STEP, 5, n); } } } +} + +//------------------------------expand_macro_nodes---------------------- +// Returns true if a failure occurred. +bool PhaseMacroExpand::expand_macro_nodes() { + if (StressMacroExpansion) { + C->shuffle_macro_nodes(); + } // Clean up the graph so we're less likely to hit the maximum node // limit diff --git a/src/hotspot/share/opto/macro.hpp b/src/hotspot/share/opto/macro.hpp index 7f27688a57a..6b8c95e2d69 100644 --- a/src/hotspot/share/opto/macro.hpp +++ b/src/hotspot/share/opto/macro.hpp @@ -203,6 +203,7 @@ public: void refine_strip_mined_loop_macro_nodes(); void eliminate_macro_nodes(); bool expand_macro_nodes(); + void eliminate_opaque_looplimit_macro_nodes(); SafePointScalarObjectNode* create_scalarized_object_description(AllocateNode *alloc, SafePointNode* sfpt); static bool can_eliminate_allocation(PhaseIterGVN *igvn, AllocateNode *alloc, GrowableArray *safepoints); diff --git a/src/hotspot/share/opto/phasetype.hpp b/src/hotspot/share/opto/phasetype.hpp index 517f0aa72c7..2bb810c55cc 100644 --- a/src/hotspot/share/opto/phasetype.hpp +++ b/src/hotspot/share/opto/phasetype.hpp @@ -91,6 +91,8 @@ flags(PHASEIDEALLOOP_ITERATIONS, "PhaseIdealLoop iterations") \ flags(AFTER_LOOP_OPTS, "After Loop Optimizations") \ flags(AFTER_MERGE_STORES, "After Merge Stores") \ + flags(AFTER_MACRO_ELIMINATION_STEP, "After Macro Elimination Step") \ + flags(AFTER_MACRO_ELIMINATION, "After Macro Elimination") \ flags(BEFORE_MACRO_EXPANSION , "Before Macro Expansion") \ flags(AFTER_MACRO_EXPANSION_STEP, "After Macro Expansion Step") \ flags(AFTER_MACRO_EXPANSION, "After Macro Expansion") \ diff --git a/src/utils/IdealGraphVisualizer/README.md b/src/utils/IdealGraphVisualizer/README.md index 195878a5b99..7f48702c769 100644 --- a/src/utils/IdealGraphVisualizer/README.md +++ b/src/utils/IdealGraphVisualizer/README.md @@ -33,7 +33,7 @@ Ideal graphs are dumped at the following points: * `N=2`: additionally, after every major phase * `N=3`: additionally, after every minor phase * `N=4`: additionally, after every loop optimization -* `N=5`: additionally, after every effective IGVN and every macro expansion step (slow) +* `N=5`: additionally, after every effective IGVN, macro elimination, and macro expansion step (slow) * `N=6`: additionally, after parsing every bytecode (very slow) By default the JVM expects that it will connect to a visualizer on the local diff --git a/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java b/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java index ccf4822e676..534ec9d2d97 100644 --- a/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java +++ b/test/hotspot/jtreg/compiler/arguments/TestStressOptions.java @@ -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 @@ -24,7 +24,7 @@ /* * @test * @key stress randomness - * @bug 8252219 8256535 8317349 8319879 8335334 + * @bug 8252219 8256535 8317349 8319879 8335334 8325478 * @requires vm.compiler2.enabled * @summary Tests that different combinations of stress options and * -XX:StressSeed=N are accepted. @@ -56,6 +56,10 @@ * compiler.arguments.TestStressOptions * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressUnstableIfTraps -XX:StressSeed=42 * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressMacroElimination + * compiler.arguments.TestStressOptions + * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+StressMacroElimination -XX:StressSeed=42 + * compiler.arguments.TestStressOptions */ package compiler.arguments; diff --git a/test/hotspot/jtreg/compiler/debug/TestGenerateStressSeed.java b/test/hotspot/jtreg/compiler/debug/TestGenerateStressSeed.java index bbcafb53460..9542e48e54e 100644 --- a/test/hotspot/jtreg/compiler/debug/TestGenerateStressSeed.java +++ b/test/hotspot/jtreg/compiler/debug/TestGenerateStressSeed.java @@ -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 @@ -30,7 +30,7 @@ import jdk.test.lib.process.ProcessTools; /* * @test * @key stress randomness - * @bug 8252219 8256535 + * @bug 8252219 8256535 8325478 * @requires vm.compiler2.enabled * @summary Tests that using a stress option without -XX:StressSeed=N generates * and logs a random seed. @@ -40,6 +40,7 @@ import jdk.test.lib.process.ProcessTools; * @run driver compiler.debug.TestGenerateStressSeed StressIGVN * @run driver compiler.debug.TestGenerateStressSeed StressCCP * @run driver compiler.debug.TestGenerateStressSeed StressMacroExpansion + * @run driver compiler.debug.TestGenerateStressSeed StressMacroElimination */ public class TestGenerateStressSeed { diff --git a/test/hotspot/jtreg/compiler/debug/TestStress.java b/test/hotspot/jtreg/compiler/debug/TestStress.java index 6678d09e649..2046488ac40 100644 --- a/test/hotspot/jtreg/compiler/debug/TestStress.java +++ b/test/hotspot/jtreg/compiler/debug/TestStress.java @@ -30,11 +30,11 @@ import jdk.test.lib.Asserts; /* * @test * @key stress randomness - * @bug 8252219 8256535 8317349 + * @bug 8252219 8256535 8317349 8325478 * @requires vm.debug == true & vm.compiler2.enabled * @requires vm.flagless * @summary Tests that stress compilations with the same seed yield the same - * IGVN, CCP, and macro expansion traces. + * IGVN, CCP, macro elimination, and macro expansion traces. * @library /test/lib / * @run driver compiler.debug.TestStress */ @@ -69,6 +69,12 @@ public class TestStress { stressSeed); } + static String macroEliminationTrace(int stressSeed) throws Exception { + return phaseTrace("StressMacroElimination", + "CompileCommand=PrintIdealPhase,*::*,AFTER_MACRO_ELIMINATION_STEP", + stressSeed); + } + static void sum(int n) { int acc = 0; for (int i = 0; i < n; i++) acc += i; @@ -84,6 +90,8 @@ public class TestStress { "got different CCP traces for the same seed"); Asserts.assertEQ(macroExpansionTrace(s), macroExpansionTrace(s), "got different macro expansion traces for the same seed"); + Asserts.assertEQ(macroEliminationTrace(s), macroEliminationTrace(s), + "got different macro elimination traces for the same seed"); } } else if (args.length > 0) { sum(Integer.parseInt(args[0])); diff --git a/test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java b/test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java index b0e5f2fda5c..ad45b1d0856 100644 --- a/test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java +++ b/test/hotspot/jtreg/compiler/lib/ir_framework/CompilePhase.java @@ -99,6 +99,8 @@ public enum CompilePhase { PHASEIDEALLOOP_ITERATIONS( "PhaseIdealLoop Iterations"), AFTER_LOOP_OPTS( "After Loop Optimizations"), AFTER_MERGE_STORES( "After Merge Stores"), + AFTER_MACRO_ELIMINATION_STEP( "After Macro Elimination Step"), + AFTER_MACRO_ELIMINATION( "After Macro Elimination"), BEFORE_MACRO_EXPANSION( "Before Macro Expansion"), AFTER_MACRO_EXPANSION_STEP( "After Macro Expansion Step"), AFTER_MACRO_EXPANSION( "After Macro Expansion"), diff --git a/test/hotspot/jtreg/testlibrary/ctw/src/sun/hotspot/tools/ctw/CtwRunner.java b/test/hotspot/jtreg/testlibrary/ctw/src/sun/hotspot/tools/ctw/CtwRunner.java index c842035183a..d721c7d63c5 100644 --- a/test/hotspot/jtreg/testlibrary/ctw/src/sun/hotspot/tools/ctw/CtwRunner.java +++ b/test/hotspot/jtreg/testlibrary/ctw/src/sun/hotspot/tools/ctw/CtwRunner.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 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 @@ -318,6 +318,7 @@ public class CtwRunner { "-XX:+StressIGVN", "-XX:+StressCCP", "-XX:+StressMacroExpansion", + "-XX:+StressMacroElimination", "-XX:+StressIncrementalInlining", // StressSeed is uint "-XX:StressSeed=" + rng.nextInt(Integer.MAX_VALUE),