RFR: 8366702: C2 SuperWord: refactor VTransform vector nodes [v4]

[Emanuel Peter]

On Wed, 10 Sep 2025 08:10:07 GMT, Galder Zamarreño <galder at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix typo
>
> src/hotspot/share/opto/vtransform.cpp line 795:
> 
>> 793: 
>> 794:   VectorNode* vn = nullptr;
>> 795:   if (req() <= 3) {
> 
> I'm wondering if with this change, the `assert(2 <= req() && req() <= 4, "Must have 1-3 inputs");` call could moved and be made more specific for these 2 sides of the conditon.
> 
> For example, we know that if we go down the `req() <= 3` route, then we're in the 1-2 inputs? And if if we're in the other one we're at least 3 inputs.
> 
> Then, with that in mind, I wonder if we couldn't move `  Node* in3 = (req() >= 4) ? apply_state.transformed_node(in_req(3)) : nullptr;` to be computed only in the `else` and convert it to `Node* in3 = apply_state.transformed_node(in_req(3))`?

We could. But I'd prefer to do the req assert before I access any inputs, to avoid failing in the input access.

And I also like the parallel pattern of fetching the inputs, moving it inside the if/else would in my opinion make it harder to read.

We could also just drop the assert and rely on the asserts in the input fetch.

Personally, I would leave it as I have it now, but I'm open to a majority vote ;)

@chhagedorn What would you prefer?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27056#discussion_r2336034124