On Tue, 2 Dec 2025 08:50:02 GMT, Paul Hübner <[email protected]> wrote:
>> In >> https://github.com/openjdk/valhalla/blob/acb511a9f5c7b750b41e1ce77aab3d1a59613097/test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java#L98-L116 >> >> we have 2 allocations: `Iter` and `Integer`. Scalar replacement allows to >> eliminate the allocation of `Iter`, but we still had the allocation of >> `Integer`. With value classes, we can also save the allocation of `Integer` >> since it is a value class. Adapting expectations is enough. >> >> To keep the test robust, I prefered to expect the exact amount, and not >> something like >> >> @IR(phase = { CompilePhase.PHASEIDEAL_BEFORE_EA }, counts = { IRNode.ALLOC, >> "<= 2" }) >> @IR(phase = { CompilePhase.ITER_GVN_AFTER_ELIMINATION }, counts = { >> IRNode.ALLOC, "<= 1" }) >> >> Since that would allow the respective counts 1 and 1 (for instance, if >> `Integer` allocation is being removed as a value class, but `Iter` is not >> because EA would be broken with Valhalla). >> >> To allow the test to work precisely with and without Valhalla, I propose a >> fake flag `enable-valhalla` that checks whether `Integer` is a value class. >> I preferred that over more generally "enable-preview" because it lacks >> granularity with respect to other preview features, and >> `PreviewFeatures.isEnabled()` is internal and not accessible anyway. It's a >> little hack, but I think the usage is very natural. It would be good if >> @chhagedorn could take a look at it. >> >> Thanks, >> Marc > > test/hotspot/jtreg/compiler/c2/irTests/scalarReplacement/ScalarReplacementWithGCBarrierTests.java > line 108: > >> 106: // could not be eliminated. >> 107: @Test >> 108: @IR(applyIf = {"enable-valhalla", "false"}, phase = { >> CompilePhase.PHASEIDEAL_BEFORE_EA }, counts = { IRNode.ALLOC, "2" }) > > With `EnableValhalla` being removed in favour of `--enable-preview`, I think > it would make sense if we transition to a preview on/off state rather than > the fine-grained one we have right now. FWIW there's also `jdk.internal.misc.PreviewFeatures` that can be used to check if preview features are enabled at runtime. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1767#discussion_r2580232418
