[Fix] Sort dataflow kernel calls so producers precede consumers - #574
Open
zzzDavid wants to merge 3 commits into
Open
[Fix] Sort dataflow kernel calls so producers precede consumers#574zzzDavid wants to merge 3 commits into
zzzDavid wants to merge 3 commits into
Conversation
Vitis HLS `#pragma HLS dataflow` requires functions to appear in canonical forward-flow order (producers before consumers). Previously, kernel call order in the generated top function followed the user's definition order, which could place a consumer (e.g. an offchip store kernel) before the processing elements that produce its stream data. This caused hw_emu to produce all-zero outputs while the simulator (which uses concurrent threads with blocking FIFOs) worked correctly. Sort kernels by stream direction: pure producers first, mixed read/write kernels (PEs) in the middle, pure consumers last. Fixes cornell-zhang#572 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a Vitis HLS dataflow/hw_emu mis-scheduling issue by changing the generated top() kernel call order so stream producers are emitted before stream consumers, rather than preserving the user’s Python definition order.
Changes:
- Adds a stream-direction-based sort of dataflow kernel call order in
_build_top(). - Updates the “last kernel” marker to reflect the new sorted order.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| return 2 # pure consumer -> last | ||
| return 1 # mixed / no streams -> middle | ||
|
|
||
| sorted_funcs = sorted(funcs, key=_stream_order_key) |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _build_top() function collected top-level arguments in the order they were first encountered across kernel functions. When a producer kernel (e.g. offchip_loadQ) referenced bidirectional arguments (read as input in one kernel, written as output in another), those args appeared before pure-input args from other kernels. This violated the Vitis HLS requirement that output arguments appear at the end, causing "Output arguments must appear at the end" errors. Fix: classify each argument by checking whether any consumer kernel uses it (making it an output). Sort so input-only args come first, output/bidirectional args come last. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_build_top(), sort kernel call order by stream direction: pure producers first, mixed read/write kernels (PEs) in the middle, pure consumers last. This satisfies Vitis HLS's#pragma HLS dataflowcanonical forward-flow ordering requirement.Root Cause
Previously, kernel calls in the generated HLS
top()function followed the user's Python definition order. If a consumer kernel appeared before the PEs that produce its data, Vitis HLS could not correctly schedule the dataflow pipeline, resulting in all-zero outputs in hw_emu (while the simulator, which uses concurrent threads with blocking FIFOs, worked correctly).Test Plan
Fixes #572
🤖 Generated with Claude Code