On Tue, 10 Nov 2026 17:24:29 GMT, Emanuel Peter <[email protected]> wrote:
>> Eric Fang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Use explicit NodeTypeCon template arguments
>
> src/hotspot/share/opto/genericopts.cpp line 37:
>
>> 35: // and PVECTMASK for PVectMask nodes. Nodes of the same NT type have
>> essentially
>> 36: // the same primitive operations, so the optimization decouples from each
>> 37: // specific node type.
>
> This seems like it promises a pretty wide "generic optimization framework".
> But for now you only use it for vector optimizations. How generic do you
> envision this to be?
It is intended for GVN/IGVN optimizations that apply to multiple node types but
cannot directly share their opcode matching, constant representation, or node
construction.
The existing node methods remain responsible for invoking the optimization.
`GenericOpts` contains the shared transformation, while `NodeOps<NT>` provides
only the operations required for a particular
node type. It is therefore not a new optimization phase or registry.
> src/hotspot/share/opto/genericopts.cpp line 43:
>
>> 41:
>> 42: template<>
>> 43: class NodeOps<NodeTypeCon::VECTOR> : public StackObj {
>
> Since this is vector related, why not move the vector specific stuff to a
> vector specific file?
Yes, that is feasible. We could separate the type-specific parts by node
family. The structure would be roughly:
// genericopts_impl.hpp
template<NodeTypeCon NT>
class NodeOps;
template<NodeTypeCon NT>
Node* GenericOpts::Ideal_XorAnd_to_AndNot(Node* n, PhaseGVN* phase) {
NodeOps<NT> ops(phase);
// Shared optimization logic.
}
// Vector-specific source file
template<>
class NodeOps<NodeTypeCon::VECTOR> {
// ...
};
template<>
class NodeOps<NodeTypeCon::PVECTMASK> {
// ...
};
template Node*
GenericOpts::Ideal_XorAnd_to_AndNot<NodeTypeCon::VECTOR>(
Node*, PhaseGVN*);
template Node*
GenericOpts::Ideal_XorAnd_to_AndNot<NodeTypeCon::PVECTMASK>(
Node*, PhaseGVN*);
// Scalar-specific source file
template<>
class NodeOps<NodeTypeCon::INT> {
// ...
};
template<>
class NodeOps<NodeTypeCon::LONG> {
// ...
};
template Node*
GenericOpts::Ideal_XorAnd_to_AndNot<NodeTypeCon::INT>(
Node*, PhaseGVN*);
template Node*
GenericOpts::Ideal_XorAnd_to_AndNot<NodeTypeCon::LONG>(
Node*, PhaseGVN*);
The common template implementation would need to be moved to an internal
implementation header, because each source file must see both the optimization
template and its `NodeOps` specialization when explicitly instantiating it.
We currently keep them in one .cpp file to avoid introducing an additional
`genericopts_impl.hpp` layer. Although the `VECTOR`, `PVECTMASK`, and future
scalar implementations would be in the same file, they are independent
specializations and are not coupled with each other. Keeping them together also
makes the implementation relatively compact.
If you think separating the node-family-specific parts into different files
makes the design clearer, I am happy to change it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32095#discussion_r4533054862
PR Review Comment: https://git.openjdk.org/jdk/pull/32095#discussion_r4533217055
