Track: hierarchical-dataflow upstream engagement (#588 + design discussion)

[sunwookim028]

Tracking the deferred hierarchical-dataflow upstream engagement. Neither item below is posted yet. Plan: engage upstream cornell-zhang#588 first with the draft comment, and float a design discussion later (anchored inside cornell-zhang#588 or attached to a concrete PR) rather than seeding a broad parallel discussion while the maintainer drives cornell-zhang#588.

Held item 1: draft comment for upstream cornell-zhang#588 ([Bug][Builder] Issues in Dataflow Hierarchical Design, by Fangtangtang)

Note before posting: the two repros were NOT locally reproduced; soften any reproduction claim before posting.

Thanks for the precise write-up. Both repros are clear and map to the same builder path.

On (1), agreed: the region body is recovered with inspect.getsource() and handed to ast.parse() still carrying the indentation of the enclosing kernel scope, so any region invoked from inside a kernel trips IndentationError. Dedenting the source, or passing the object straight to parse_ast as you suggest, is the right quick fix. The trigger is specifically a sub-region call sitting inside a kernel body rather than at the top level.

On (2), also agreed. Each region instantiation re-emits a func.func under the same mangled name (vadd__0 at both call sites), so the second instantiation collides. Emitting the region function once and calling it N times, or making the instantiation suffix unique per call site, both resolve it. The emit-once option also matches how the kernels are already lowered as callable functions, so it is probably the smaller change.

One framing, if useful: these two look like the same seam rather than two unrelated bugs. The builder currently assumes a kernel is an isolated unit and a region is emitted once in place, so both a sub-region invoked from within a kernel body and the same region instantiated more than once are cases the front end does not yet model. I have been keeping notes on exactly this region-composition seam (nested invocation, region reuse, region-scope shared state) from the HLS side, and would be glad to share them or help with a fix plus regression tests. Happy to send a PR for the two quick fixes above.

Held item 2: draft for a hierarchical-dataflow design discussion

Status: HELD. Category and title style still TBD (house style is bracketed [Area] titles in the General category; plain "Hierarchical dataflow architecture" was the owner's initial preference). Accurate issue mapping: cornell-zhang#561 -> faces 1 and 2; cornell-zhang#565 -> face 2; cornell-zhang#577 review -> face 4; face 3 (EmitVivadoHLS.cpp per-kernel static) has no dedicated issue.

Draft title: Hierarchical dataflow architecture

Draft body:
Both of our on-device accelerators (allo-tpu on U280/XRT and its allo-npu sibling) need the same shape: one @df.region that owns shared state (scratchpad, accumulator, instruction memory) and streams, with a decoder kernel and a driver kernel that concurrently read and write that shared state. This is the canonical HLS dataflow topology (Vitis_Libraries, FINN-hlslib, AutoSA, ScaleHLS all put shared accelerator state at the top scope and run PEs as concurrent sub-functions). It is what any programmable 2D mesh design lowered through Allo needs.

Building it today surfaces four issues that share one root cause: a @df.kernel is modeled as an IsolatedFromAbove function, so the region body cannot own state and channels that concurrent kernels share. The four faces:

1. Simulator: sub-region calls inside control flow leave callee streams un-lowered. Tracked in [Bug] Hierarchical regions with inter-kernel streams deadlock in simulator and have HLS codegen issues cornell-zhang/allo#561; deadlock half fixed in fix: hierarchical dataflow simulator deadlock and HLS codegen (#561, #565) cornell-zhang/allo#577.
2. Front-end: region-scope @stateful shared across inner kernels does not survive ASTContext.copy(). Tracked in [Bug] Missing global_op_cache on ASTContext.copy() cornell-zhang/allo#565 (closed via fix: hierarchical dataflow simulator deadlock and HLS codegen (#561, #565) cornell-zhang/allo#577). Shared region-scope state still not in the upstream test corpus; [Bug][Builder] Issues in Dataflow Hierarchical Design cornell-zhang/allo#588 shows hierarchical-builder bugs remain.
3. Back-end: EmitVivadoHLS.cpp emits each stateful global as a static array inside every kernel body, so two kernels sharing one stateful buffer mis-compiles. Not yet addressed upstream.
4. Interface: auto-captured scalar control ports get no s_axilite emission; the fix: hierarchical dataflow simulator deadlock and HLS codegen (#561, #565) cornell-zhang/allo#577 review rejected scalars in args=[...], pushing users onto auto-capture with no AXI-Lite path. Workaround is int32[1] bound as m_axi. Not yet addressed upstream.

Roadmap question: should @df.region stay a grouping of isolated kernels with state grafted on, or become a hierarchical module that owns state and channels, with kernels as concurrent processes and explicit interface bindings? The second is the industry-standard mesh model and dissolves all four faces into one design. Related live threads: cornell-zhang#588, cornell-zhang#592, cornell-zhang#524.

Next action

Post held item 1 to upstream cornell-zhang#588 (after softening the reproduction wording), then decide on the discussion venue.

[sunwookim028]

Design backup (2026-07-15) from notes/HIERARCHY_DESIGN.md: open questions and architecture alternatives

Backup of sections 7 and 8 of notes/HIERARCHY_DESIGN.md (drafted 2026-05-12),
copied essentially verbatim ahead of a notes/ cleanup pass. The file itself
stays in the repo as the full design record; this comment preserves the two
forward-looking sections on the tracking issue. Note (2026-07-15): items 1 and
2 discussed in the file (simulator nested-call streams, region-scope Stateful
crash) have since landed upstream via PR cornell-zhang#577 (merged 2026-05-13); the
questions and alternatives below are still open for items 3 and 4 and for the
broader "what should @df.region want to be" design question.

7. Open questions for the future session (Sunwoo, pre-pitch)

1. What does @df.region want to be? A grouping of independently-isolated kernels (today's model, with grafted state) or a hierarchical module owning state and channels (industry-standard mesh model)? Pick one explicitly and let the design follow. Without this, the maintainers will continue to merge local fixes that compound technical debt.
2. Is region-scope Stateful actually compatible with IsolatedFromAbove? The fork's stateful_var_map propagation makes it work via name-based symbol lookup, not direct SSA use. That's fine for MLIR, but does it generalize to AIE / Tapa / Catapult backends, or just LLVM-sim + vhls? Pitch should not promise something only the two backends support.
3. Should the AXI-Lite control plane be a first-class concept, or a vhls-only postprocess? Today s_axilite lives in postprocess_hls_code (a string-substitution layer). If it becomes first-class (a new MemorySpace / Bundle attribute on region args), every backend has to honor it. Worth it?
4. Do upstream maintainers know about the decoder + driver HLS pattern? Probably not in the form allo-tpu/allo-npu use it - feather and the hierarchical mesh tests both avoid it. Pitch should include a one-page motivation showing "this is the canonical HLS coding pattern, not an edge case". Cite Vitis_Libraries (DATAFLOW + hls::stream as the production canonical form), FINN-HLSLib (hierarchical PE mesh in production HLS), and AutoSA or ScaleHLS (precedents for high-level to HLS mesh-PE lowering) as evidence that the pattern is industry-standard at the HLS code level.
5. What's the migration story? If items 1-4 land as a unified design, existing code using Stateful at kernel scope still works (kernel-local Stateful is the only thing exercised upstream). What about args=[] with int32[1]? Does it stay as a legacy path, or get rewritten by a pass?
6. Who owns the RFC review? Fangtangtang reviewed fix: hierarchical dataflow simulator deadlock and HLS codegen (#561, #565) cornell-zhang/allo#577 (regions/HLS), chhzh123 is Allo's PI. The RFC should be addressed to both, and probably to hc676 (Hongzheng Chen, who authored [dataflow] Support hierarchical modules cornell-zhang/allo#518) as the architect of the current hierarchical model.

8. Suggested architectural alternatives to weigh

Three sketches, not full designs. The future session should pick one as the pitch's centerpiece.

Alt A: Region-as-Module (industry-aligned)

@df.region becomes a SystemC-style hierarchical module:

• Region body owns Streams and Stateful arrays as module-scope state.
• Each @df.kernel is a concurrent process; access to region state is by name (no args= for state, only for top-level interface ports).
• Top-level interface ports get explicit binding attributes: @s_axilite("ctrl"), @m_axi(bundle="gmem0"). No more auto-capture-vs-args asymmetry.
• HLS emits one C++ function per kernel, one static T X[N]; per Stateful at file scope, one top wrapper with the right pragmas.

Pros: matches Vitis HLS/Catapult/SystemC, all four items dissolve into a single redesign. Cons: bigger PR, breaks existing args=[A,B,C] ergonomics for new users.

Alt B: Capture-by-name without isolation

Keep @df.kernel as a function, but drop IsolatedFromAbove for region-scope state symbols. Region-scope Stateful and scalars become captured automatically; kernels reference them by name. The MLIR side becomes a pre-pass that lowers captures into per-kernel arg lists just before the kernel functions are emitted.

Pros: minimal user-facing change; preserves the current kernel-as-function vocabulary. Cons: keeps the structural mismatch - every backend has to know how to lower captures, items 1/3 still need their own fixes (the items become smaller but don't dissolve).

Alt C: Single-decl multiple-use Stateful + df.scalar

Smallest viable change: keep the current model; add (i) a module-level pass that ensures each __stateful_* symbol is emitted at file scope exactly once in vhls, (ii) a df.scalar(int32) type that explicitly binds to s_axilite. Items 1 and 2 remain as separate bugfixes; items 3 and 4 dissolve.

Pros: easiest to land. Cons: doesn't resolve the structural mismatch - the next hierarchical-region project hits the same class of bugs in a different corner. Buys 6 months at most.

Recommended path: pitch Alt A as the long-horizon design, propose Alt C as the bridge-the-gap interim if maintainers want to land things in two weeks instead of two months. Alt B is the worst of both worlds and probably not worth pitching.