Skip to content

[executorch][native] Add Graph index arena to the in-memory IR - #22280

Open
SS-JIA wants to merge 1 commit into
gh/SS-JIA/617/basefrom
gh/SS-JIA/617/head
Open

[executorch][native] Add Graph index arena to the in-memory IR#22280
SS-JIA wants to merge 1 commit into
gh/SS-JIA/617/basefrom
gh/SS-JIA/617/head

Conversation

@SS-JIA

@SS-JIA SS-JIA commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Stack from ghstack (oldest at bottom):

Adds ptn::Graph, the index arena that owns the value layer landed so far. It
is a pure function body (mirrors the schema Graph): it holds the Nodes and
Values that the NodeId / ValueId handles point into, the ordered graph
input / output value lists, and -- recursively -- the subgraph bodies for
higher-order ops. Stateful method-level bindings (constants, output specs,
mutable buffers) are deferred to a later Method type.

Design decisions baked in:

  • Per-graph subgraph arena. Graph owns std::vector<Graph> subgraphs and
    GraphId indexes the enclosing graph's subgraphs, matching the schema's
    recursion and the per-Graph SSA namespace so a subgraph stays self-contained
    with its parent. (A std::vector<Graph> member of Graph is legal C++17 --
    the standard containers permit an incomplete value type at the point of the
    member declaration.)

  • Storage identity vs execution order are decoupled. nodes is an
    append-only arena so a NodeId never shifts (the index-arena invariant),
    while schedule (std::vector<NodeId>) carries the topological / execution
    order a runtime walks. At load the arena order equals the wire's topological
    order and schedule is the identity [0, n) (initialize_schedule()); across
    mutation the arena order is no longer topological, so schedule is
    authoritative -- reorder / insert there (moving int32s, invalidating no id)
    rather than moving storage. Deletion via tombstone + a compacting pass is
    deferred until a mutating pass needs it.

  • Pure arena. inputs / outputs are ValueId lists (schema SSA names
    resolved to ids at deserialize); no tensor_values side table in memory
    (each Value already carries its TensorMeta); the name to id map stays
    deserializer-local.

rebuild_def_use() recomputes every Value's producer / consumers from the
nodes (order-independent -- it walks the arena, not schedule). Placeholder and
Output nodes are real entries in nodes, so def-use is uniform: a graph input
value's producer is its placeholder node, and graph inputs are identified by
membership in inputs, not by producer == kInvalid. This corrects the now
stale Value.h producer comment (also in this diff). Bounds-checked
node() / value() / subgraph() accessors throw on an invalid id, and the
IR printer gains to_string(const Graph&), a multi-line dump in schedule
order.

consumer_ids is a set of consuming nodes rather than a bag of uses, so
add(x, x) lists its consumer once and size() counts consumers. Since nodes
are walked in arena order a repeated operand appends consecutively, so a tail
check keeps that exact without a lookup structure.

rebuild_def_use() distinguishes the two things an unusable id can mean.
kInvalid is an absent operand and is skipped; an id that is set but does not
address the value arena can only be a corrupt graph, and now throws rather than
being skipped, which would have left def-use half-wired with no signal. That
matches the accessors, which already throw on a bad id.

Pure std only (no ExecuTorch, no flatbuffers), consistent with the rest of the
standalone ptn runtime.

Differential Revision: D114396767

[ghstack-poisoned]
@pytorch-bot

pytorch-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22280

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 50bc327 with merge base a5f15b5 (image):

FLAKY - The following job failed but was likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 28, 2026

@digantdesai digantdesai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants