On Wed, 11 Mar 2026 13:42:30 GMT, Per Minborg <[email protected]> wrote:
> Implement JEP 531: Lazy Constants (Third Preview) > > This PR replaces _draft_ https://github.com/openjdk/jdk/pull/29507 Thank you for working on this, @minborg! I only reviewed the IR-test. The test itself looks good. I merely have some style suggestions we like to follow for compiler tests. test/hotspot/jtreg/compiler/c2/irTests/stable/LazyConstantsIrTest.java line 1: > 1: /* Could you please move this to `compiler/stable`. We consider the `irTests` folder as a mistake and want to avoid putting more tests there. test/hotspot/jtreg/compiler/c2/irTests/stable/LazyConstantsIrTest.java line 26: > 24: /* > 25: * @test > 26: * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" > | os.arch=="amd64" Suggestion: * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.simpleArch=="x64" I would prefer the simplified version. test/hotspot/jtreg/compiler/c2/irTests/stable/LazyConstantsIrTest.java line 28: > 26: * @requires os.arch=="aarch64" | os.arch=="riscv64" | os.arch=="x86_64" > | os.arch=="amd64" > 27: * @requires vm.gc.Parallel > 28: * @requires vm.compiler2.enabled Suggestion: This is not required anymore (see [JDK-8380594](https://bugs.openjdk.org/browse/JDK-8380594)). test/hotspot/jtreg/compiler/c2/irTests/stable/LazyConstantsIrTest.java line 33: > 31: * @library /test/lib / > 32: * @enablePreview > 33: * @run main compiler.c2.irTests.stable.LazyConstantsIrTest Suggestion: * @run driver ${test.class.main} IR tests require the driver VM and we prefer the new JTREG variable to prevent typos and enable moving tests more easily. test/hotspot/jtreg/compiler/c2/irTests/stable/LazyConstantsIrTest.java line 98: > 96: } > 97: > 98: } Suggestion: } Nit: some git tools start complaining when the trailing newline is missing. ------------- Changes requested by mhaessig (Committer). PR Review: https://git.openjdk.org/jdk/pull/30194#pullrequestreview-3997105108 PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r2979747436 PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r2979774334 PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r2979778948 PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r2979783740 PR Review Comment: https://git.openjdk.org/jdk/pull/30194#discussion_r2979790687
