On Thu, 29 Oct 2026 04:23:23 GMT, Eric Fang <[email protected]> wrote:
>> 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.
>
> Hi @merykitty, I'd like to discuss this a bit further.
>
> Another reason for having Vector and VectorMask share an implementation was
> that some VectorMask nodes, such as `XorVMaskNode`, `OrVMaskNode`, and
> `AndVMaskNode`, inherit from their corresponding vector nodes (`XorVNode`,
> `OrVNode`, and `AndVNode`). From the node inheritance perspective, a vector
> mask is therefore also a Vector. On platforms such as NEON and AVX2, which do
> not have `pVectMask`, vector masks are effectively represented as vectors. So
> the type distinction between vectors and vector masks is not entirely
> clear-cut, perhaps partly because of how C2 defines these nodes. I'm not
> opposed to separating them; just wanted to clarify a bit more on the original
> approach.
>
> Also, around line 53 of this file, we define a fixed set of interfaces. The
> original intention was to require all `NodeTypeCon` types to implement these
> interfaces for consistency. However, vectors and vector masks support quite
> different sets of operations, and some optimizations may not be shareable
> across all types. Given that, it seems reasonable not to require every type
> to implement the same set of interfaces. So, should we require all types to
> implement a fixed set of interfaces? I'd be interested to hear your thoughts
> on this.
>
> Thanks~
@erifan I think there are some misconception here:
> some `VectorMask` nodes, such as `XorVMaskNode`, `OrVMaskNode`, and
> `AndVMaskNode`, inherit from their corresponding vector nodes (`XorVNode`,
> `OrVNode`, and `AndVNode`).
Similar to other node inheritance in C2 (such as `OrINode` inherits `AddNode`
for some reasons), I think this is bad design, no inheritence guideline would
guide you towards a `XorVMaskNode` being a `XorVNode`. As a result, we should
not rely on such design for the design of this optimization.
> On platforms such as NEON and AVX2, which do not have `pVectMask`, vector
> masks are effectively represented as vectors.
You are confused a `j.i.v.VectorMask` with a `PVectMask`. A `j.i.v.VectorMask`
is an abstract concept provided by the Vector API, it is either mapped to a
`PVectMask` on architectures that support it, or a `Vect` otherwise. On the
other hand, a `PVectMask` is a value that is represented by by a predicate
register, and is entirely different from a `Vect`.
> Also, around line 53 of this file, we define a fixed set of interfaces. The
> original intention was to require all `NodeTypeCon` types to implement these
> interfaces for consistency. However, vectors and vector masks support quite
> different sets of operations,
That actually further adds reasons why we should have different `NodeTypeCon`
for `Vect` and `PVectMask`. Since if you incorrectly call an optimization that
is not available on `PVectMask`, the code will not compile. You don't need to
implement all these interfaces, it should only implement the necessary one. For
example, since `PVectMask` does not support addition, if you try to call
`GenericOpts::Ideal_Add<VECTOR_MASK>`, the template instantiation will fail.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32095#discussion_r4408865569
