On Mon, 28 Apr 2025 06:45:58 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> This is not specifically required on x86, but because this test fails on x86 >> when `-XX:UseAVX=0` is specified. When `-XX:UseAVX=0` is specified, the >> sub-graph is like this: >> `(XorV (VectorMaskCmp (LoadVector ...)) (Replicate -1))` >> It is not an optimization pattern supported by this patch because we don't >> know what's the comparison op. > > I would also prefer if you added the IR restrictions rather than the JTREG > requires. > The benefit is that we can still run the tests on all platforms, at least for > result verification. > > Imagine someone adds optimizations to a new platform, but does not know about > this test here. They make a mistake, and there is a bug, leading either to a > crash or wrong result. With the requires, you test would never even run, and > we would not catch it. With the IR applyIf, we would catch the bug. I can make the change, it's not complex, but it is different from what I thought before. I thought that supporting vector was the default behavior, is it right? So when I was doing an architecture-independent feature or optimization, I should just exclude those unsupported cases from the test, so that all potential environments would be tested. If I was doing an architecture- or feature-dependent optimization, then I should limit the test to run only in supported environments. For this case, **the current meaning of @requires is "skip this test when -XX:UseAVX=0 is specified on the x86 architecture, otherwise run the tests".** So if a new architecture (say s390) supports related vector operations in the future, then this test will be run on that platform by default. If all architecture-independent tests are restricted with applyIfCPUFeatureOr, then when we support a new architecture, we will need to modify all tests, otherwise no test will run on this architecture. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2063076640
