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

[Xiaohong Gong]

On Fri, 9 Jan 2026 11:03:47 GMT, Emanuel Peter <epeter 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.
>
> 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?

Yes, correct.

> 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.

OK, I will remove these lines. Thanks for your suggestion!

> 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?

Sure. I will add for it in next commit.

> 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.

Yeah, I remember that @PaulSandoz  gave a suggestion for the name before. I will take a consideration. Renaming of these two IRs is not an easy task that we need to go through all the code in mid-end, and backend of platforms that have supported Vector API. I'd like to leave it as a separate task with this PR. WDYT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2680926486
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2680927789
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2680928497
PR Review Comment: https://git.openjdk.org/jdk/pull/29130#discussion_r2680943534