RFR: 8302681: [IR Framework] Only allow cpuFeatures from a verified list [v3]

[Tobias Hartmann]

On Tue, 21 Feb 2023 12:06:11 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Wrongly typed cpuFeatures can lead to an IR rule being ignored.
>> Example: https://github.com/openjdk/jdk/pull/12601 [JDK-8302668](https://bugs.openjdk.org/browse/JDK-8302668)
>> 
>> We should have a list of verified cpuFeatures, and assert if one is used that is not in that list. That way, typos and bad copies can be avoided, and we make sure the tests run when intended.
>> 
>> I verified the `intel` cpuFeatures by hand on my laptop. And for `asimd` and `sve` I asked @tobiasholenstein and @nick-arm .
>> 
>> `sve1` seems to have been a typo, and I am changing it to `sve`.
>> 
>> Note: cpu-features can be displayed like this: `./java -Xlog:os+cpu --version`
>> 
>> On my machine, I get:
>> 
>> cmov, fxsr, ht, mmx, 3dnowpref, sse, sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, lzcnt, tsc, tscinvbit, avx, avx2, aes, erms, clmul, bmi1, bmi2, adx, avx512f, avx512dq, avx512cd, avx512bw, avx512vl, sha, fma, vzeroupper, avx512_vpopcntdq, avx512_vpclmulqdq, avx512_vaes, avx512_vnni, clflush, clflushopt, clwb, avx512_vbmi2, avx512_vbmi, rdtscp, rdpid, fsrm, gfni, avx512_bitalg, f16c, pku, ospke, cet_ibt, cet_ss, avx512_ifma
>> 
>> 
>> @nick-arm got this on one of his machines:
>> 
>> fp, asimd, evtstrm, aes, pmull, sha1, sha256, crc32, lse, dcpop, sha3, sha512, sve, paca
>> 
>> 
>> @tobiasholenstein got this:
>> 
>> fp, asimd, aes, pmull, sha1, sha256, crc32, lse, sha3, sha512
>> 
>> 
>> I had to adapt a test that used a non-existent cpu feature. I replaced it with an impossible combination.
>
> Emanuel Peter has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - add comment
>  - modified test that used non-existent cpu feature

> I would like to keep the list small, and separate from other lists. It has to be clear that this list is verified, and if one adds something one has to be very sure that it is not a typo or non-existent.

Okay, fine with me.

`TestPreconditions.java` needs a copyright update.

test/hotspot/jtreg/compiler/lib/ir_framework/test/IREncodingPrinter.java line 246:

> 244:         List<String> verifiedCPUFeatures = new ArrayList<String>( Arrays.asList(
> 245:             // General
> 246:             "sha3",

Isn't `sha3` AArch64 specific?

test/hotspot/jtreg/compiler/lib/ir_framework/test/IREncodingPrinter.java line 247:

> 245:             // General
> 246:             "sha3",
> 247:             "fma",

I would put this under `x86` or something.

test/hotspot/jtreg/compiler/lib/ir_framework/test/IREncodingPrinter.java line 261:

> 259:             "avx512vl",
> 260:             "avx512f",
> 261:             // Arm

Suggestion:

            // AArch64

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestPreconditions.java line 48:

> 46:     public static void testApplyIfOnly() {}
> 47: 
> 48:     // The IR check should not be applied, since asimd is arm and sse intel.

Suggestion:

    // The IR check should not be applied, since asimd is aarch64 and sse intel.

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestPreconditions.java line 54:

> 52:     public static void testApplyIfCPUFeatureOnly() {}
> 53: 
> 54:     // The IR check should not be applied, since asimd is arm and sse intel.

Suggestion:

    // The IR check should not be applied, since asimd is aarch64 and sse intel.

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

Changes requested by thartmann (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12669