RFR: 8332402: [IR Framework] Add tests for applyIfCPUFeature* and applyIfPlatform* in TestBadFormat

Christian Hagedorn chagedorn at openjdk.org
Mon May 27 08:41:25 UTC 2024


On Mon, 20 May 2024 08:27:33 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you help to review this patch to add some test for IR test framework?
> As discussed https://github.com/openjdk/jdk/pull/19270#pullrequestreview-2060974799, it's worth to add some tests for for applyIfCPUFeature* and applyIfPlatform* in TestBadFormat.
> 
> Thanks

Thanks for adding all these new tests! I have some comments.

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestBadFormat.java line 865:

> 863:     @IR(failOn = IRNode.CALL, applyIfCPUFeature = {"sve", "true", "avx", "true"})
> 864:     @IR(failOn = IRNode.CALL, applyIfCPUFeature = {"sve", "true", "avx"})
> 865:     public void applyIfCPUFeatureTooManyFlags() {}

Suggestion:

    public void applyIfCPUFeatureTooManyCPUFeatures() {}


I suggest to replace `flag(s)` in all the new tests by `CPUFeature(s)`

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestBadFormat.java line 867:

> 865:     public void applyIfCPUFeatureTooManyFlags() {}
> 866: 
> 867:     @FailCount(4)

Why is this different from `applyIfMissingValue()`? Also, there are some other tests where the `FailCount` value is different from the corresponding flag-test version.

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

PR Review: https://git.openjdk.org/jdk/pull/19302#pullrequestreview-2080225426
PR Review Comment: https://git.openjdk.org/jdk/pull/19302#discussion_r1615620913
PR Review Comment: https://git.openjdk.org/jdk/pull/19302#discussion_r1615638289


More information about the hotspot-compiler-dev mailing list