On Tue, 27 Oct 2026 09:38:18 GMT, Quan Anh Mai <[email protected]> wrote:
>> There are three possible approaches here:
>>
>> 1. Keep `VECTOR` and `PVECTMASK` completely separate, with two macros and
>> two separate `NodeOpsTraits` implementations.
>> - Pros:
>> - - The templates for `VECTOR` and `PVECTMASK` are instantiated at
>> compile time.
>> - - Decouple `vector` and `pvectormask` so they can evolve
>> **independently** in the future.
>> - Cons: Some code duplication.
>>
>> 2. Make no distinction between them (the current implementation).
>> - Pros: A single `NodeOpsTraits` implementation.
>> - Cons: Requires distinguishing between vector and vector mask types at
>> runtime.
>>
>> 3. Define separate `VECTOR` and `PVECTMASK` enum values, but share a single
>> `NodeOpsTraits` implementation. Add an extra template parameter,
>> `IS_PVECTMASK`, and specify its value when specializing for a node.
>> - Pros: Compile-time instantiation.
>> - Cons: An additional layer of template instantiation.
>>
>> So far, there don’t seem to be any **shared** optimizations where we cannot
>> distinguish between `vector` and `pvectormask`. Vector mask nodes typically
>> inherit from vector nodes; for example, `XorVMaskNode` inherits from
>> `XorVNode`. So for simplicity, I chose **option 2**. WDYT?
>
> It is less about writing and more about reading the code. In general, it is
> easier to reason about when the boundary is clear. It is also easier to read:
>
> static bool is_and(const Node* n) {
> return n->Opcode() == Op_AndV && !n->is_predicated_vector();
> }
>
> than:
>
> static bool is_and(const Node* n) {
> return !n->is_predicated_vector() &&
> n->Opcode() == (is_pvectmask(n) ? Op_AndVMask : Op_AndV);
> }
>
> In fact, we should default to seperation, and only sharing when there is a
> compelling reason to do so.
So, you prefer **option 1**, right? I agree that is **clearer**.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32095#discussion_r4387189481
