On Thu, 23 Mar 2023 07:13:55 GMT, Xiaohong Gong <xg...@openjdk.org> wrote:
>> Quan Anh Mai has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 14 commits: >> >> - move implementations up >> - Merge branch 'master' into shufflerefactor >> - Merge branch 'master' into shufflerefactor >> - reviews >> - missing casts >> - clean up >> - fix Matcher::vector_needs_load_shuffle >> - fix internal types, clean up >> - optimise laneIsValid >> - Merge branch 'master' into shufflerefactor >> - ... and 4 more: https://git.openjdk.org/jdk/compare/d063b896...a4835c00 > > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java > line 868: > >> 866: return (Byte128Vector) Byte128Vector.VSPECIES.dummyVector() >> 867: .vectorFactory(s.indices()); >> 868: } > > Move the implementation details to the super class? I have moved `toBitsVectorTemplate` to `AbstractShuffle`, also `toShuffle` has been refactored to be more versatile. > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteMaxVector.java > line 862: > >> 860: v.convertShape(VectorOperators.B2I, species, 3) >> 861: .reinterpretAsInts() >> 862: .intoArray(a, offset + species.length() * 3); > > Can we add a method like `intoIntArray()` in `ByteVector` and move these > common code there? The same to other vector types. I don't think this and the below suggestion achieve much, reducing the generated code at the expense of moving the usage away from the definition does not seem worth it to me. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13093#discussion_r1153353267 PR Review Comment: https://git.openjdk.org/jdk/pull/13093#discussion_r1153354836