On Fri, 14 Nov 2025 15:59:18 GMT, Emanuel Peter <[email protected]> wrote:
>> Hamlin Li has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - add CMove+CmpP/N tests >> - fix cmovF/D_cmpP > > test/hotspot/jtreg/compiler/c2/irTests/TestScalarConditionalMove.java line 36: > >> 34: * @test >> 35: * @summary Test conditional move. >> 36: * @requires vm.simpleArch == "riscv64" > > I would prefer if you could enable the test on all platforms, but just > require the specific platform on the IR rules. > What would be even more fantastic: if you were able to also enable the IR > rules for `x64` and `aarch64`, but we can also file a follow-up RFE for that. Make sense. I filed https://bugs.openjdk.org/browse/JDK-8371920 to track the task, will do it later after this pr. > test/hotspot/jtreg/compiler/c2/irTests/TestScalarConditionalMove.java line 49: > >> 47: "-XX:+UnlockExperimentalVMOptions", >> "-XX:-UseCompactObjectHeaders"); >> 48: TestFramework.runWithFlags("-XX:+UseCMoveUnconditionally", >> "-XX:-UseVectorCmov", >> 49: "-XX:+UnlockExperimentalVMOptions", >> "-XX:+UseCompactObjectHeaders"); > > Wait. Is this just a copy of the existing vector test, but run with CMove > vectorization disabled? > If so, we could just add these additional runs to the existing test, and > guard the IR test with corresponding flags: > Have an IR rule for `-XX:-UseVectorCmov` and one for `-XX:+UseVectorCmov`. > > That would allow us to reduce some code duplication. And it would also avoid > letting the two tests go out of sync when people add more to one but not the > other. > > What do you think? Good idea! I can do it. What do you think about the name of the merged tests? `TestConditionalMove.java` or `TestScalarAndVectorConditionalMove.java` ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2528463608 PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2528467634
