RFR: 8342095: Add autovectorizer support for subword vector casts
Emanuel Peter
epeter at openjdk.org
Tue Feb 4 18:33:12 UTC 2025
On Mon, 3 Feb 2025 04:40:23 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:
> Hi all,
> This patch adds initial support for the autovectorizer to generate conversions between subword types. Currently, when superword sees two packs that have different basic types, it discards them and bails out of vectorization. This patch changes the behavior to ask the backend if a cast between the conflicting types is supported, and keeps the pack if it is. Later, when the `VTransform` graph is built, a synthetic cast is emitted when packs requiring casts are detected. Currently, only narrowing casts are supported as I wanted to re-use existing `VectorCastX2Y` logic for the initial version, but adding more conversions is simple and can be done with a subsequent RFE. I have attached a JMH benchmark and got these results on my Zen 3 machine:
>
>
> Baseline Patch
> Benchmark (SIZE) Mode Cnt Score Error Units Score Error Units Improvement
> VectorSubword.intToByte 1024 avgt 12 200.049 ± 19.787 ns/op 56.228 ± 3.535 ns/op (3.56x)
> VectorSubword.intToShort 1024 avgt 12 179.826 ± 1.539 ns/op 43.332 ± 1.166 ns/op (4.15x)
> VectorSubword.shortToByte 1024 avgt 12 245.580 ± 6.150 ns/op 29.757 ± 1.055 ns/op (8.25x)
>
>
> I've also added some IR tests and they pass on my linux x64 machine. Thoughts and reviews would be appreciated!
Great work, looks generally amazing 😊
I left a few comments below.
src/hotspot/share/opto/vtransform.hpp line 525:
> 523: };
> 524:
> 525: class VTransformCastNode : public VTransformNode {
I think it would be good to make it a `VTransformCastVectorNode`.
test/hotspot/jtreg/compiler/loopopts/superword/TestSubwordVectorization.java line 53:
> 51: for (int i = 0; i < SIZE; i++) {
> 52: res[i] = RANDOM.nextInt();
> 53: }
Can you please use `Generators.java`? It would be great if we can use that in the future, to create more "interesting" input data ;)
test/hotspot/jtreg/compiler/loopopts/superword/TestSubwordVectorization.java line 101:
> 99: @IR(applyIfCPUFeature = { "avx", "true" },
> 100: applyIfOr = {"AlignVector", "false", "UseCompactObjectHeaders", "false"},
> 101: counts = { IRNode.VECTOR_CAST_I2B, IRNode.VECTOR_SIZE_ANY, ">0" })
Hmm. It would be great if we could assert the length of the vector. Otherwise we may get less than we could.
Something like this may work:
`IRNode.VECTOR_SIZE + "min(max_int, max_byte)"`
Have you tried that yet?
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java line 154:
> 152: @IR(applyIfCPUFeature = { "avx", "true" },
> 153: applyIfOr = {"AlignVector", "false", "UseCompactObjectHeaders", "false"},
> 154: counts = { IRNode.VECTOR_CAST_I2S, IRNode.VECTOR_SIZE + "min(max_int, max_short)", ">0" })
Ah see, here we have the vector size asserted, good!
test/hotspot/jtreg/compiler/vectorization/runner/ArrayTypeConvertTest.java line 165:
> 163: @Test
> 164: @IR(failOn = {IRNode.STORE_VECTOR})
> 165: // Subword vector casts do not work currently, see JDK-8342095.
There seem to be other cases in this file that mention `JDK-8342095`. We should probably file a new RFE for those, right?
test/micro/org/openjdk/bench/vm/compiler/VectorSubword.java line 73:
> 71: }
> 72: }
> 73: }
Ah, these are all casting to smaller types. What about casting to larger types?
-------------
PR Review: https://git.openjdk.org/jdk/pull/23413#pullrequestreview-2593704858
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941692475
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941693919
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941700026
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941700871
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941703352
PR Review Comment: https://git.openjdk.org/jdk/pull/23413#discussion_r1941701910
More information about the hotspot-compiler-dev
mailing list