RFR: 8317349: Randomize order of macro node expansion in C2

[Christian Hagedorn]

On Mon, 5 Feb 2024 14:02:55 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> This changeset adds a flag `StressMacroExpansion` which randomizes macro expansion ordering for stress testing purposes.
>> 
>> Changes:
>> - Implement the flag `StressMacroExpansion`.
>> - Add functionality tests for `StressMacroExpansion`, analogous to testing for previous stress flags such as `StressIGVN`.
>> - Add `StressMacroExpansion` to a number of tests that already turn on all other stress flags (and therefore should also use `StressMacroExpansion`).
>> - Rename the previous compiler phase `MACRO_EXPANSION` to `AFTER_MACRO_EXPANSION`.
>> - Add new Ideal phases related to macro expansion, allowing `StressMacroExpansion` testing. Below is a sample phase list screenshot including some of the new phases (IGV print level 4).
>>    <p align="center">
>>       <img src="https://github.com/openjdk/jdk/assets/4222397/b65c67a0-13f7-45a6-aef4-5dfe259dab66" width="300">
>>    </p>
>> 
>> 
>> Question for the review: are the new macro expansion Ideal phases OK, or should we change anything?
>> 
>> Testing:
>> - [GitHub Actions](https://github.com/dlunde/jdk/actions/runs/7756242938)
>> - tier1 to tier5 on windows-x64, linux-x64, linux-aarch64, macosx-x64, and macosx-aarch64.
>> - Tested that thousands of graphs are correctly opened and visualized with IGV.
>
> src/hotspot/share/opto/macro.cpp line 2517:
> 
>> 2515:       assert(!success || (C->macro_count() == (old_macro_count - 1)), "elimination must have deleted one node from macro list");
>> 2516:       progress = progress || success;
>> 2517:       if (success) { C->print_method(PHASE_AFTER_MACRO_EXPANSION_ELIMINATE, 4, n); }
> 
> I suggest to put it on a new line. 
> Suggestion:
> 
>       if (success) { 
>         C->print_method(PHASE_AFTER_MACRO_EXPANSION_ELIMINATE, 4, n); 
>       }
> 
> I think dumping at level 4 (same for the other newly added dumps) is too verbose already but when dumping at level 5, you have the overhead of additional IGVN step dumps. I could imagine that you either want macro expansion dumps OR IGVN step dumps but not always both. So, it looks like we've now reached a point where it would have been nice to enable/disable certain dumps. 
> 
> Anyway, since this is not possible I suggest to move these dumps to level 5 for now. But I'm open for other opinions about that.

I think we should also add this to `eliminate_macro_nodes()` when eliminating locks, allocations, and boxing nodes. But then again, we also dump the graphs directly after EA since `eliminate_macro_nodes()` is also called from there. Not sure if that's wanted. I guess it's okay being at IGV dump level 5. But we could also restrict the dumps to the post loop opts phase.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17684#discussion_r1478327532