On Fri, 14 Nov 2025 18:12:56 GMT, Hamlin Li <[email protected]> wrote:
>> 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`
`TestConditionalMove.java` sounds good :)
It would also be nice if we could move it out of the `irTests` directory, we
would like to eventually move all tests away from it, and rather sort the tests
by what they test and not by how we test them. Though now it's a little tricky
because we check for both vector and scalar things. Still, I would propose that
you move it under `c2/vectorization` or `c2/loopopts/superword`, since they do
include vectorization tests. An alternative could also be in a new `c2/cmove`
directory.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2533020809
