RFR: 8351952: [IR Framework]: allow ignoring methods that are not compilable [v3]

[Christian Hagedorn]

On Tue, 18 Mar 2025 09:39:01 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> With the Template Framework, I'm generating IR tests randomly. But random code can always hit bailouts in compilation, and make code not compilable any more. We should have a way to disable this check, and just gracefully continue to execute the tests.
>> 
>> To allow a single test method to be `not compilable`:
>> https://github.com/openjdk/jdk/blob/ce40f1402387f75ea8627883979e3cbf63480941/test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java#L160-L161
>> 
>> To allow all test methods to be `not compilable`:
>> https://github.com/openjdk/jdk/blob/ce40f1402387f75ea8627883979e3cbf63480941/test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java#L140-L144
>> 
>> See also this documentation in the code:
>> https://github.com/openjdk/jdk/blob/ce40f1402387f75ea8627883979e3cbf63480941/test/hotspot/jtreg/compiler/lib/ir_framework/Test.java#L88-L94
>> 
>> ---------------------------------------
>> 
>> **Backrgound**
>> 
>> My random code seems to hit a bailout in the Register Allocator, and I cannot do much to predict if that bailout happens.
>> See https://bugs.openjdk.org/browse/JDK-8304328
>
> Emanuel Peter has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Merge branch 'JDK-8351952-IR-Framework-not-compilable' of https://github.com/eme64/jdk into JDK-8351952-IR-Framework-not-compilable
>  - Update test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestPhaseIRMatching.java
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>
>  - more for Christian

Some small test comments, otherwise, it looks good now. Thanks for the updates!

Maybe to share some background about our offline discussions: Conceptionally, it would be better if the test VM will adjust the IR encoding when a method is not compilable and it is expected. Then the driver VM does not even need to know about that case and can apply IR matching as it would normally do. However, that turned out to be not very easy to implement. It would require some more extensive refactorings which are out of scope. We might still want to address them at some point but that should be done separately. We decided it's the easiest way to handle it in IR matching for now.

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java line 100:

> 98:         try {
> 99:             framework.start();
> 100:             throw new RuntimeException("should have thrown TestRunException or IRViolationException");

To be more explicit since `TestRunException` is only thrown in the test VM which is then noticed by the driver VM and rethrown with a `TestVMException`:
Suggestion:

            throw new RuntimeException("should have thrown TestRunException/TestVMException or IRViolationException");

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java line 102:

> 100:             throw new RuntimeException("should have thrown TestRunException or IRViolationException");
> 101:         } catch (TestVMException e) {
> 102:             // Happens when we hit the issue during explicit compilabion by the Framework.

Suggestion:

            // Happens when we hit the issue during explicit compilation by the Framework.

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java line 123:

> 121:         try {
> 122:             framework.start();
> 123:             throw new RuntimeException("should have thrown TestRunException");

Same here:
Suggestion:

            throw new RuntimeException("should have thrown TestRunException/TestVMException");

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24049#pullrequestreview-2693718687
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000620528
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000613990
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000622191