RFR: 8317349: Randomize order of macro node expansion in C2 [v2]

[Christian Hagedorn]

On Mon, 5 Feb 2024 15:38:34 GMT, Daniel Lundén <dlunden 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.
>
> Daniel Lundén has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove flag from some test cases
>  - Updates after Christian's comments

Thanks for the updates!

> Regarding whether to randomize macro node elimination, I find it a bit confusing that PhaseMacroExpand also eliminates macro nodes, perhaps it would be better to, in a separate RFE, split the phase into PhaseMacroEliminate and PhaseMacroExpand, and introduce a separate StressMacroElimination option and new phase names for the former?

I agree with that. A new phase would be better. Let's do that in a separate RFE.

> My suggestion: set dump levels to 5 for now, as Christian suggests, but do not yet add any dumps to eliminate_macro_nodes. I'll create a separate RFE with Roberto's suggestion (which I think sounds good and, looking at the code, quite easy to achieve through a bit of restructuring) together with IGV dumping for macro elimination. What do you think?

Sounds good!

test/hotspot/jtreg/compiler/loopopts/TestPeelingSkeletonPredicateInitialization.java line 2:

> 1: /*
> 2:  * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.

Can be undone now:
Suggestion:

 * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17684#pullrequestreview-1864669057
PR Review Comment: https://git.openjdk.org/jdk/pull/17684#discussion_r1479468819