RFR: 8370666: VectorAPI: Add clear comments for vector mask relative code in c2

[Emanuel Peter]

On Fri, 9 Jan 2026 01:36:50 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

> The VectorMask implementation in Vector API involves complex interactions between types, nodes, and platform-specific
> features, making the related code in HotSpot difficult to understand and review.
> 
> This patch adds comprehensive comments for vector mask related types, nodes, and methods in C2 to improve code clarity and
> maintainability.
> 
> Note: This patch only adds comments; no functional changes are made.

Nice work, thanks for taking the time for this, much appreciated!

On the whole I'm super happy with this, but left a few extra comments :)

src/hotspot/share/opto/type.cpp line 2452:

> 2450: //   stored in a predicate/mask register.
> 2451: // - Returns a normal vector type (i.e. TypeVectA ~ TypeVectZ) otherwise, where
> 2452: //   the vector mask is stored in a vector register.

The first case is `PVectMask`, and the second `NVectMask`, right?

src/hotspot/share/opto/vectornode.hpp line 81:

> 79: // the masked nodes.
> 80: //
> 81: // For example, "AddVBNode" might have two versions:

Might?
Suggestion:

// For example:

src/hotspot/share/opto/vectornode.hpp line 1478:

> 1476: };
> 1477: 
> 1478: //-------------------------- Vector mask broadcast ------------------------------

A nit that has been bothering me for a while:

I would just remove all the "title lines" with the `------`.
They don't really add anything. And they are currently inconsistent in this file anyway.
Some are a more of a description like here. Some repeat the node name. And in some cases they are missing anyway.

src/hotspot/share/opto/vectornode.hpp line 1739:

> 1737:   VectorRearrangeNode(Node* vec1, Node* shuffle)
> 1738:     : VectorNode(vec1, shuffle, vec1->bottom_type()->is_vect()) {
> 1739:     // assert(mask->is_VectorMask(), "VectorBlendNode requires that third argument be a mask");

Can you add a comment for Rearrange as well?

src/hotspot/share/opto/vectornode.hpp line 1841:

> 1839: 
> 1840: // Select elements from two source vectors based on the wrapped indexes held in
> 1841: // the first vector.

Can you improve the documentation a little? Also: "first vector" might be understood to refer to `src1`, but that's not the case, right?

You could base the description on `selectFrom`:
https://download.java.net/java/early_access/jdk26/docs/api/jdk.incubator.vector/jdk/incubator/vector/FloatVector.html#selectFrom(jdk.incubator.vector.Vector,jdk.incubator.vector.Vector)

src/hotspot/share/opto/vectornode.hpp line 1855:

> 1853: //------------------------------VectorLoadShuffleNode------------------------------
> 1854: // The target may not directly support the rearrange operation for an element type.
> 1855: // In those cases, we can transform the rearrange into a different element type.

Can you  specify a bit more about the inputs and outputs, and what exactly the transformation does?

src/hotspot/share/opto/vectornode.hpp line 1874:

> 1872: // Convert a "BVectMask" into a platform-specific vector mask (either "NVectMask"
> 1873: // or "PVectMask").
> 1874: class VectorLoadMaskNode : public VectorNode {

I'd love to rename this. Because it is (as you say in the comments) a conversion, and not a "load" (memory op).
What about `VectorConvertBooleans2MaskNode`.

And below, rename `VectorStoreMaskNode` to `VectorConvertMask2BooleansNode`.

You may have an even better idea.

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

PR Review: https://git.openjdk.org/jdk/pull/29130#pullrequestreview-3643441539
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2675764381
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2675755496
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2676211590
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2676174171
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2676190790
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2676195417
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2676205674