RFR: 8366427: C2 SuperWord: refactor VTransform scalar nodes [v2]

Manuel Hässig mhaessig at openjdk.org
Mon Sep 1 07:00:44 UTC 2025


On Mon, 1 Sep 2025 06:56:51 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
>> [See plan overfiew.](https://bugs.openjdk.org/browse/JDK-8340093)
>> 
>> This is a pure refactoring - no change in behaviour. I'm presenting it like this because it will make reviews easier.
>> 
>> The goal is to split up some cases that are currently treated the same, but will alter have different behavior. There may be a little bit of code duplication, but the code will soon be made different ;)
>> 
>> We split the `VTransformScalarNode`:
>> - `VTransformMemopScalarNode`
>>   - Uses that only wanted scalar mem nodes can now directly check for `isa_MemopScalar`.
>>   - We can directly store the `_vpointer` in a field, that way we don't need to do a lookup via `vloop_analyzer`. This could also be helpful later on if we ever do widening (unrolling during auto vectorization): we could then do the necessary modifications to the `vpointer`.
>> - `VTransformLoopPhiNode`
>>   - Later on, they will play a more special role, they will give us easy access to the beginning state of the loop body and the backedges.
>> - `VTransformCFGNode`
>>   - Calling them scalar nodes is not 100% accurate. We'll probably have to further refine them later on. But splitting them off now seems like a reasonable choice. Once we do if-conversion we'll have to do more work on CFG.
>> - `VTransformDataScalarNode`
>>   - These represent all the normal "calculation" nodes in the loop.
>> - `VTransformInputScalarNode` -> `VTransformOuterNode`:
>>   - For now, we are still just tracking input nodes, but soon we will need to track input and output nodes: basically just the 1-hop neighbourhood of nodes outside the loop. I'm already renaming them now, so it will be less noise later.
>> 
>> I decided to rather split up more, and avoid the `VTransformScalarNode` together, avoiding having to override overrides - that can be really confusing (e.g. what I had with `is_load_in_loop`).
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update src/hotspot/share/opto/vtransform.hpp
>   
>   Co-authored-by: Manuel Hässig <manuel at haessig.org>

Thank you for your continued efforts, @eme64. The suspense is building for your big change...

This looks good to me, bar one typo.

Marked as reviewed by mhaessig (Committer).

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

> 452: };
> 453: 
> 454: // Identity ransform for scalar loads and stores.

Suggestion:

// Identity transform for scalar loads and stores.

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

Marked as reviewed by mhaessig (Committer).

PR Review: https://git.openjdk.org/jdk/pull/27002#pullrequestreview-3172282140
PR Review: https://git.openjdk.org/jdk/pull/27002#pullrequestreview-3172310479
PR Review Comment: https://git.openjdk.org/jdk/pull/27002#discussion_r2313027649


More information about the hotspot-compiler-dev mailing list