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

[Emanuel Peter]

On Tue, 18 Mar 2025 08:40:53 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   documentation from Christian
>
> test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 418:
> 
>> 416:      * true, which allows any test to pass even if there is a compilation bailout. If only selected methods are prone
>> 417:      * to bail out, it is preferred to use {@link Test#allowNotCompilable()} instead for more fine-grained control.
>> 418:      * By setting this flag, any associated {@link IR} rule of a test is only executed if the test method was compiled, 
> 
> Whitespace error.

fixed

> test/hotspot/jtreg/compiler/lib/ir_framework/test/AbstractTest.java line 122:
> 
>> 120:             tryCompileMethod(test);
>> 121:         } catch (MethodNotCompilableException e) {
>> 122:             TestRun.check(test.isAllowNotCompilable(), "Only allowNotCompilable methods should throw MethodNotCompilableException.");
> 
> Should we emit a log message here in case we have an expected compilation bailout here?

done!

> test/hotspot/jtreg/compiler/lib/ir_framework/test/AbstractTest.java line 175:
> 
>> 173:         }
>> 174:         TestRun.check(WHITE_BOX.isMethodCompilable(testMethod, test.getCompLevel().getValue(), false),
>> 175:                       "Method " + testMethod + " not compilable (anymore) at level " + test.getCompLevel());
> 
> You now check `isMethodCompilable()` twice. You could move the second check inside the `catch` block for `MethodNotCompilableException`. Then you can use this exception whenever there is a method that is not compilable (as the class name suggests) and then only do the separation between allowed and disallowed cases there.

done!

> test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestNotCompilable.java line 102:
> 
>> 100:             throw new RuntimeException("should have thrown TestRunException");
>> 101:         } catch (TestVMException e) {
>> 102:         } catch (IRViolationException e) {}
> 
> This is a mismatch. Which exceptions are now expected? Same below.

Adjusted and added comment :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000590741
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000590141
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000590272
PR Review Comment: https://git.openjdk.org/jdk/pull/24049#discussion_r2000590601