On Tue, 10 Dec 2024 07:01:27 GMT, Jatin Bhateja <jbhat...@openjdk.org> wrote:
>> Quan Anh Mai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> adverb order
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte512Vector.java
> line 1046:
>
>> 1044: String msg = ("index "+si+"out of range
>> ["+length+"] in "+
>> 1045: java.util.Arrays.toString(indices));
>> 1046: throw new AssertionError(msg);
>
> Why not directly throw IndexOutOfBoundsException here?
This is called in an `assert` so I think throwing `AssertionError` seems more
reasonable, the original implementation also throws `AssertionError`
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Double128Vector.java
> line 859:
>
>> 857: .reinterpretAsInts()
>> 858: .intoArray(a, offset);
>> 859: default -> {
>
> These cases for length() = 4, 8, and 16 looks redundant for 128-bit
> DeoubleVector.
You are right, but this switch is needed for `DoubleMaxVector`, and using it
for the other species simplifies the template. The compiler will eliminate all
wrong cases so there should be no runtime concern.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21042#discussion_r1877563187
PR Review Comment: https://git.openjdk.org/jdk/pull/21042#discussion_r1877559527
