On Mon, 28 Apr 2025 07:46:14 GMT, erifan <d...@openjdk.org> wrote:

>> 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.

Hmm, I'm not convinced that vector operations are supported by default. Every 
platform supports a different set.

In your case, you so far only have negative rules, i.e. you expect certain 
nodes NOT to be generated / in the final IR. I suppose in that case you can 
assert that you NEVER get those nodes, because if you have vectors not 
supported, they will not show up because of that, and if you do support 
vectors, they should be optimized away.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24674#discussion_r2063752141

Reply via email to