On Fri, 17 Apr 2026 07:31:07 GMT, Emanuel Peter <[email protected]> wrote:

>> Found while working on JDK-8369699.
>> 
>> In general, the Vector API documentation is a bit vague around part numbers.
>> There was considerable confusion around expansion/contraction: are we 
>> talking about logical, physical or output expansion/contraction? 
>> Confusingly, in some places we called expansions things that go from more to 
>> fewer bits, and we called contractions that went from fewer to more bits.
>> 
>> And exception messages are not very helpful, for example they don't provide 
>> the legal range.
>> 
>> @rose00 took a first stab at improving things 
>> (https://github.com/openjdk/jdk/pull/29306), and I eventually took over the 
>> project.
>> 
>> --------------------------------------
>> 
>> Principles:
>> - Expansion means fewer->more bits.
>> - Contraction means more->fewer bits.
>> - Be clear about input, logical result and output.
>> - We primarily use:
>>   - conversion lanewise expansion (logical)
>>   - conversion lanewise contraction (logical)
>>   - conversion lanewise in-place (logical)
>>   - reinterpret (logical)
>>   - select, for truncation (output)
>>   - insert, for padding (output)
>>   - in-place, logical fits output (output)
>> 
>> Please review this PR in this order:
>> - Changes in the "Expansions, contractions, and partial results" section of 
>> `Vector.java`. We must first agree on the definitions here, before we go and 
>> disagree elsewhere ;)
>> - Changes in affected methods `convertShape`, `convert`, and 
>> `reinterpretShape`.
>> - Internal changes in `AbstractVector.java‎`: adjust nomenclature and 
>> exception message.
>> - New test. I think it is necessary, I caught some mistakes I made. And when 
>> I wanted to add tests for `unslice` I realized that it does not throw for 
>> out of bounds `part`. So I think it is justified.
>> 
>> In general, I'm a bit worried that the documentation is a bit too long, and 
>> feels a bit heavy/overwhelming.
>> To a large degree, this is due to the complexity of part numbers.
>> We could drop some paragraphs and some repetition. Let me know what you 
>> think is too much.
>> More explanations may help make things clearer, but also risk being too much 
>> and overwhelming.
>> I'm open to cut things down more, and any other constructive suggestions ;)
>> 
>> -----------------------
>> 
>> While reading the documentation and testing for `unslice`, I found out that 
>> https://github.com/openjdk/jdk/pull/3804 accidentally removed the bounds 
>> checks for `part`. But we did not notice, because there were no tests for 
>> it. I'm adding the bounds check back in and adding tests ...
>
> Emanuel Peter has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   after merge with master: fix fuzzer for unslice out of bounds exception

Very thorough improvements since I saw this last.

The main explanations of expansion/contraction are rephrased and rebalanced 
well.

This is a much more rigorous presentation of "what is going on here", which 
clarifies the explanation of "why it’s like this".

Good tactical repetition around per-method mentions of `Vector.html#expansion`.

I like the introduction of the term "exact fit" to amplify the term "in-place".

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Vector.java line 
955:

> 953:  * contracting lane size ({@code ML=1/8}),
> 954:  * shape-invariant ({@code MP=1}).
> 955:  * We must insert the logical resut {@code byte[2]:16} into

s/resut/result/

These are good examples to prepare the reader for the big table that follows.

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

Marked as reviewed by jrose (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/30113#pullrequestreview-4174259189
PR Review Comment: https://git.openjdk.org/jdk/pull/30113#discussion_r3140959573

Reply via email to