RFR: 8366357: C2 SuperWord: refactor VTransformNode::apply with VTransformApplyState

Christian Hagedorn chagedorn at openjdk.org
Thu Aug 28 13:35:46 UTC 2025


On Thu, 28 Aug 2025 12:57:44 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> I'm working on **cost-modeling**, and am integrating some smaller changes from this proof-of-concept PR: https://github.com/openjdk/jdk/pull/20964
> 
> This is a **pure refactoring** - no change in behaviour. I'm presenting it like this because it will make reviews easier.
> 
> The goal here is that `VTransformNode::apply` only needs a single argument. This is important, as we will soon add more components that need to be updated during apply. That way, we can simply add more parts to `VTransformApplyState`, and do not need to add more arguments to VTransformNode::apply.
> 
> And yes: I have considering passing the `apply_state` as `const`. While this may be possible with the current code state, the upcoming changes from https://github.com/openjdk/jdk/pull/20964 will require non-const access to the `apply_state` (e.g. for `set_memory_state`).

Two small suggestions, otherwise, looks good!

src/hotspot/share/opto/vtransform.cpp line 737:

> 735:   // bits in a scalar shift operation. But vector shift does not truncate, so
> 736:   // we must apply the mask now.
> 737:   Node* shift_count_masked = new AndINode(shift_count_in, phase->igvn().intcon(_mask));

Randomly noticed this: You should probably use `phase->intcon()` which also sets root as control`. Same at other places further down. You might want to squeeze that into this patch as well.

src/hotspot/share/opto/vtransform.hpp line 273:

> 271:   // generated def (input) nodes when we are generating the use nodes in "apply".
> 272:   GrowableArray<Node*> _vtnode_idx_to_transformed_node;
> 273: 

Suggestion:

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/26987#pullrequestreview-3164852132
PR Review Comment: https://git.openjdk.org/jdk/pull/26987#discussion_r2307425745
PR Review Comment: https://git.openjdk.org/jdk/pull/26987#discussion_r2307407693


More information about the hotspot-compiler-dev mailing list