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

[Emanuel Peter]

On Wed, 10 Sep 2025 08:46:44 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> 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?

I discussed a bit with @chhagedorn .

He thought I could move down the `Node* in3 = apply_state.transformed_node(in_req(3))`.

Maybe if we extend the element wise ops to cases with yet another input it will have to be moved up again, but it's fine to move down for now.

The assert we'll leave where it is, it makes more sense as a precondition. As such, I'll move it to the top of the method.

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

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