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