On Thu, 12 Mar 2026 02:49:09 GMT, Vladimir Ivanov <[email protected]> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review comments resolution
>
> src/hotspot/share/opto/vectornode.cpp line 1293:
> 
>> 1291: // scalar operation.
>> 1292: //
>> 1293: // VectorOperation (VectorBroadcast INP1) (VectorOperation 
>> (VectorBroadcast INP2) INP3) =>
> 
> The comment looks confusing: it mentions `VectorBroadcast` while the 
> corresponding node is named `ReplicateNode`.

Addressed

> src/hotspot/share/opto/vectornode.hpp line 158:
> 
>> 156:   static int opcode(int sopc, BasicType bt);         // scalar_opc -> 
>> vector_opc
>> 157:   static int scalar_opcode(int vopc, BasicType bt);  // vector_opc -> 
>> scalar_opc, 0 if not handled
>> 158:   static Node* make_scalar(Compile* c, int sopc, Node* control, Node* 
>> in1, Node* in2, Node* in3);
> 
> It's a bit weird to see `VectorNode::make_scalar()`. It can be either moved 
> to `Node` or accept vector opcode and do vector->scalar opcode conversion 
> internally. 
> 
> Also, it would be nice to ensure that `VectorNode::opcode()` and 
> `VectorNode::scalar_opcode()` agree.  And `VectorNode::make_scalar()` can be 
> one place where it is checked (`assert(opcode(scalar_opcode(vopc)) == vopc, 
> "%s", NodeClassNames[vopc])`).

Addressed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25617#discussion_r2930360793
PR Review Comment: https://git.openjdk.org/jdk/pull/25617#discussion_r2930359976

Reply via email to