RFR: 8337876: [IR Framework] Add support for IR tests with @Stable

[Aleksey Shipilev]

On Tue, 6 Aug 2024 11:43:32 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> It is currently not possible to write IR tests with `@Stable` annotations because one need to somehow add the IR test classes to the boot classpath. This patch provides support to write such IR tests. I've added a section to the README to provide guidance on how this can be done.
> 
> This is motivated by https://github.com/openjdk/jdk/pull/19635.
> 
> I've tested this patch by taking the current patch of https://github.com/openjdk/jdk/pull/19635, dropping the `RestrictStable` flag and modifying the tests to work with the new IR framework feature:
> 
> TestFramework testFramework = new TestFramework();
> testFramework
>         .addFlags("-XX:-TieredCompilation",
>                   "-XX:+UseParallelGC")
>         .addTestClassesToBootClassPath()
>         .start();
> 
> 
> Thanks,
> Christian

Looks okay, I just think we don't need to mention `@Stable`.

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 161:

> 159: 
> 160: ### 2.5 IR Tests with `@Stable` annotation
> 161: To run tests with `@Stable` annotations, one need to add the test classes to the boot classpath. This can easily be achieved by calling `TestFramework.addTestClassesToBootClassPath()` on the test framework object:

I think mentioning `@Stable` here is overly specific. This guidance applies to any code that expects to be run in privileged mode, whether it is `@Stable`, `@Contended`, `@ReservedStackAccess`, etc. I say it should be e.g. "2.5 IR Tests With Privileged Classes" or some such.

test/hotspot/jtreg/compiler/lib/ir_framework/TestFramework.java line 329:

> 327:     /**
> 328:      * Add test classes to boot classpath. This adds all classes found on path {@link jdk.test.lib.Utils#TEST_CLASSES}
> 329:      * to the boot classpath with "-Xbootclasspath/a". This is useful when trying to run tests with @Stable annotations.

Again, `@Stable` is overly specific here. This is "just" about the privileged code.

test/hotspot/jtreg/compiler/lib/ir_framework/driver/TestVMProcess.java line 100:

> 98:         String bootClassPath = "-Xbootclasspath/a:.";
> 99:         if (testClassesOnBootClassPath) {
> 100:             // Add test classes themselves to boot classpath. This is required, for example, for IR tests with @Stable.

Here too, drop the mention of `@Stable`?

test/hotspot/jtreg/compiler/lib/ir_framework/driver/TestVMProcess.java line 107:

> 105:         cmds.add("-XX:+WhiteBoxAPI");
> 106:         // Ignore CompileThreshold and CompileCommand flags which have an impact on the profiling information.
> 107:         List<String> jtregVMFlags = Arrays.stream(Utils.getTestJavaOpts()).filter(s -> !s.contains("CompileThreshold")).toList();

This hunk seems unnecessary? I would like to have IR Framework patch that is easily backportable and does not contain additional, non-essential hunks :)

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

PR Review: https://git.openjdk.org/jdk/pull/20477#pullrequestreview-2221121777
PR Review Comment: https://git.openjdk.org/jdk/pull/20477#discussion_r1705425371
PR Review Comment: https://git.openjdk.org/jdk/pull/20477#discussion_r1705425878
PR Review Comment: https://git.openjdk.org/jdk/pull/20477#discussion_r1705426506
PR Review Comment: https://git.openjdk.org/jdk/pull/20477#discussion_r1705428312