Allow explicit marking of local variables as the key for merge explosion with `CompilerDirectives.mergeExplodeKey` #12557

blaumeise20 · 2025-11-18T13:18:25Z

This PR adds a method CompilerDirectives.mergeExplodeKey which can be used to explicitly mark variables to be used as the merge key in MERGE_EXPLODE loop explosion.

Currently, the entire frame state is used for merging. Taking Espresso as an example, if you would jump to the same bci but assign a different value to another variable, it will result in two separate code paths. This can lead to unexpected graph size explosion and bugs. With the method introduced in this PR, you can mark a single variable to be used as the key. Truffle will merge on this key only, but verify that other variables stay the same. If merging should occur but there is a mismatch, the compilation will bail out.

Future steps include allowing multiple variables to be marked simultaneously (which has to be extended into irreducible loop handling), and creating an option for explicit PHI nodes, so local variables can be used as counters without the need for objects.

blaumeise20 · 2025-11-18T13:18:44Z

@gilles-duboscq

gilles-duboscq

Please also check the 2 failed gates

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/CompilerDirectives.java

....graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java

gilles-duboscq · 2025-11-20T11:14:01Z

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/LoopExplosionKeyNode.java

+import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
+
+@NodeInfo(nameTemplate = "LoopExplosionKey", cycles = CYCLES_0, size = SIZE_0)
+public final class LoopExplosionKeyNode extends FloatingNode implements Canonicalizable {


What happens if the directive is used in a non-merge-explode method and this node remains in the graph?
I think this should either be a very explicit compilation failure or performance warning. If it's only a warning then we need to remove such nodes.

@chumer what do you think? Should using CompilationDirective.mergeExplodeKey outside of the context of a @ExplodeLoop(kind = LoopExplosionKind.MERGE_EXPLODE) method be a compilation error or a performance warning?

I would go with a compilation error. We can do a similar thing as in VerifyFrameDoesNotEscapePhase.
Register the phase in TruffleTier.

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java

...esso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/BytecodeNode.java

chumer · 2025-12-01T14:59:58Z

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/CompilerDirectives.java

+     * @param i the variable that should be used as the merge key.
+     * @return the unchanged value
+     *
+     * @since 25.1


Please add an entry about this in the truffle changelog.md. Including a migration recommendation that if you use ExplodeLoop MERGE that you should also use this new API.

What issue ID should I specify?

You can use GR-71613 ("Allow explicitly selecting the value used for merging in MERGE_EXPLODE")

truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/CompilerDirectives.java

chumer · 2025-12-01T15:06:34Z

....graal.compiler/src/jdk/graal/compiler/truffle/substitutions/TruffleGraphBuilderPlugins.java

+                     * lose the node when encoding the graph. At the time of decoding,
+                     * `canDelayIntrinsification` will be false, so the actual node is created.
+                     */
+                    return false;


That is a good thought, but I don't think it is necessary here to delay the intrinisification.
We delay it for isPartialEvaluationConstant because the interpretation will be different if the argument is a constant and it is only really a PE constant during decoding. For this intrinsic it does not make a difference as we unconditionally wrap the value into LoopExplosionKeyNode. So I think it would actually be better always do this during parsing already, as a method once parsed is decoded many times, so we avoid the overhead of applying the intrinsic here. Probably a minor effect on compile speed, but small things add up and its also simpler this way.

I couldn't manage to keep the node alive until decoding. It would always vanish, even though I explicitly made canonicalize return this. This was the only solution I found that worked. Do you have any idea why the node could have been deleted?

Out of my expertise. Maybe @davleopo knows?

chumer · 2025-12-01T15:08:58Z

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/LoopExplosionKeyNode.java

+import jdk.graal.compiler.nodes.spi.CanonicalizerTool;
+
+@NodeInfo(nameTemplate = "LoopExplosionKey", cycles = CYCLES_0, size = SIZE_0)
+public final class LoopExplosionKeyNode extends FloatingNode implements Canonicalizable {


I would go with a compilation error. We can do a similar thing as in VerifyFrameDoesNotEscapePhase.
Register the phase in TruffleTier.

chumer · 2025-12-01T15:15:31Z

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java

+        while (methodScope.currentLoopScope != null) {
+            /* Process nodes of loop. */
+            while (!methodScope.currentLoopScope.nodesToProcess.isEmpty()) {
+                // TODO(blaumeise20): use MethodScope instead of LoopScope here and set currentLoopScope directly


TODOs must never link to user names but either link Jira or Github issues.

I saw quite a few TODOs with a username or even no assignment at all, so I thought it would be fine... What should I put here instead?

Well it's still unclear to me as part of which change you want to do this and why.
FYI there is:

GR-71753 Allow multiple explicit keys in MERGE_EXPLODE

GR-71752 Allow marking variables explicitly for phi node creation in MERGE_EXPLODE

Thanks! The way this is currently handled isn't particularly beautiful and I was thinking of actually setting the currentLoopScope inside processNextNode. I already got a few changes (not done yet) that I would add as part of the next step. Should I specify GR-71752 then?

I saw quite a few TODOs with a username

It is only a strongly enforced rule for the Truffle team, not necessarily for the compiler team. I think it is good practice regardless.

Should I specify GR-71752 then

That should be fine

chumer · 2025-12-01T15:18:29Z

compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/nodes/GraphDecoder.java

+                keyNode.replaceAndDelete(keyNode.value);
+            }
+        }
+        if (mergeKey != null && mergeKey.size() > 1) {


Please add a comment that if mergeKey is null we do what we previously did and that is there for compatibility reasons if the merge key is not yet used.

I'll add it in the method below where the actual values are read from the frame state, this here is just the check for multiple marked variables.

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 18, 2025

gilles-duboscq reviewed Nov 20, 2025

View reviewed changes

blaumeise20 added 5 commits November 25, 2025 10:48

Formatting cleanup

acc364a

Add merge explode key marking

1138d21

Use CompilerDirectives.mergeExplodeKey in Espresso

3685fcd

Reorganize fields of MethodScope and LoopScope

a6da164

Remove unused method clearNode

911f48b

blaumeise20 force-pushed the alra/merge-explode-key branch from d9701be to 911f48b Compare November 25, 2025 09:48

oubidar-Abderrahim assigned gilles-duboscq Nov 26, 2025

chumer self-requested a review December 1, 2025 14:58

chumer requested changes Dec 1, 2025

View reviewed changes

chumer requested a review from davleopo December 1, 2025 15:19

Allow explicit marking of local variables as the key for merge explosion with CompilerDirectives.mergeExplodeKey #12557

Are you sure you want to change the base?

Allow explicit marking of local variables as the key for merge explosion with CompilerDirectives.mergeExplodeKey #12557

Uh oh!

Conversation

blaumeise20 commented Nov 18, 2025

Uh oh!

blaumeise20 commented Nov 18, 2025

Uh oh!

gilles-duboscq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Allow explicit marking of local variables as the key for merge explosion with `CompilerDirectives.mergeExplodeKey` #12557

Allow explicit marking of local variables as the key for merge explosion with `CompilerDirectives.mergeExplodeKey` #12557