[code-reflection] RFR: Reduce number of local variables when transforming code model builder to AST

[Paul Sandoz]

On Wed, 23 Apr 2025 23:20:11 GMT, Mourad Abbay <mabbay at openjdk.org> wrote:

> For CODE_BUILDER option, we transform code model builder to equivalent AST. We were introducing local variables for very expressions, as a result we were closer to the limit on local variables. In this PR, we add local variables where it's needed.

This fits nicely into the current code.

Looking more closely. You don't explicitly traverse the root expressions graphs, you just use the root values, traverse  (equivalently) on what they depend on per the op's operands via `opToTree`, and prune using the mappings in `valueToTree`, which all makes sense.

We can avoid creation of the expression graphs. Instead, you can traverse the model identifying values that are the value roots i.e., those for the invocations as we do now *and* those with 0 or > 1 uses, and accumulate them in a list, called say `rootValues`.

I think the explicit expression graph generation was useful (for me at least) to help better understand the problem and visualize but we don't need it anymore. However, the concept/pattern remains.

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/CodeModelToAST.java line 78:

> 76:         }
> 77:     }
> 78: 

Use a switch expression.

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/CodeModelToAST.java line 124:

> 122:                 Op.Result.class, Op.class);
> 123:         final MethodRef BLOCK_BUILDER_PARAM = MethodRef.method(Block.Builder.class, "parameter",
> 124:                 Block.Parameter.class, TypeElement.class);

Move to static final fields.

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/CodeModelToAST.java line 138:

> 136:                             // Variable declarations modeling local variables
> 137:                             case CoreOp.VarOp vop -> vop.operands().get(0) instanceof Op.Result;
> 138:                             // An operation result with no uses or more than one

Update comment

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/CodeModelToAST.java line 144:

> 142: 
> 143:         var stats = new ListBuffer<JCTree.JCStatement>();
> 144:         // instead, we will traverse the root expr graphs

instead of what? Perhaps say "Traverse the root expr graphs instead of traversing the model"

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/CodeModelToAST.java line 254:

> 252:                     java.util.List<Node<Value>> edges = new ArrayList<>();
> 253:                     for (Value operand : op.result().dependsOn()) {
> 254:                         // Ignore edge for operand if also used by other operations

Update comment

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

PR Review: https://git.openjdk.org/babylon/pull/410#pullrequestreview-2788971845
PR Review Comment: https://git.openjdk.org/babylon/pull/410#discussion_r2057075028
PR Review Comment: https://git.openjdk.org/babylon/pull/410#discussion_r2057075924
PR Review Comment: https://git.openjdk.org/babylon/pull/410#discussion_r2057076112
PR Review Comment: https://git.openjdk.org/babylon/pull/410#discussion_r2057085253
PR Review Comment: https://git.openjdk.org/babylon/pull/410#discussion_r2057082183