Add verify file for yaml and asm initial check #23
Conversation
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
- Fix histogram calculation: address 6 count issue resolved - Remove PE-specific debug logs (Core(2,2), Core(2,3), etc.) - Improve backpressure logging with type descriptions - Add BACKPRESSURE_TYPES.md documentation - Update analyze_all.py with friendly backpressure type names - Unify driver.go Tick() style with Core.Tick() (accumulative pattern) - Fix ICMP_EQ instruction to use parseAndCompareI - Remove unused runICMP_EQ function - Clean up commented debug code Test results: Histogram output matches expected [4, 4, 3, 8, 0, 1]
- Remove samples/ directory (replaced by test/ directory) - Update core/program_test.go to use histogram.yaml from Zeonica_Testbench - Change test to use t.Skipf instead of t.Fatalf for missing test files
…rence implementation
…play, and zoom functionality
…e English guide - Merged 6 separate markdown files into one ANALYSIS_TOOLS_GUIDE.md - Content includes: Quick Start, Tools Overview, Detailed Usage, Interactive Debugger, Debugging Workflows, FAQ, Tips - Removed: DEBUG_TOOLS_SUMMARY.md, INTERACTIVE_DEBUGGER_QUICK_REF.md, DEBUG_TOOLS_INDEX.md, CGRA_DEBUG_GUIDE.md, INTERACTIVE_DEBUGGER_FEATURES.md, IMPLEMENTATION_SUMMARY.md - Single comprehensive reference for all analysis tools
…op restart in core.go
Contributor
There was a problem hiding this comment.
Copilot reviewed 27 out of 34 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
verify/lint.go:1
- Replace magic number 1000 with a named constant (e.g.,
MaxTimesteps) to make the code more maintainable and document why this limit exists.
verify/lint.go:1 - The coordinate transformation logic (e.g., NORTH means y-1) appears inconsistent with config/platform.go's coordinate system where NORTH is top row (y=height-1, array index 0). Verify that the mesh topology assumptions are consistent across all modules to avoid confusion.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| // toTitleCase is defined in emu.go and shared across the core package |
There was a problem hiding this comment.
[nitpick] Add a comment explaining what toTitleCase does (e.g., 'converts strings to title case like strings.Title but compatible with newer Go versions') to help readers understand its purpose without needing to look at emu.go.
Suggested change
| // toTitleCase is defined in emu.go and shared across the core package | |
| // toTitleCase converts strings to title case (like strings.Title) but is compatible with newer Go versions. |
Comment on lines
+807
to
+811
| func (fs *FunctionalSimulator) readFromNeighbor(x, y int, portDir string) core.Data { | ||
| // In functional model, we don't model network delays or buffering | ||
| // For now, return default value (in practice, would fetch from neighbor) | ||
| return core.NewScalarWithPred(0, false) | ||
| } |
There was a problem hiding this comment.
readFromNeighbor always returns invalid data (Pred=false), which may cause incorrect simulation results when operations depend on network communication. Either implement actual neighbor reads or document this as a known limitation in the README.
| def build_dataflow_graph(log_file): | ||
| """ | ||
| Build a dataflow graph from log file for backward tracing. | ||
| Returns a dict mapping (cycle, from_pe, direction, channel) -> (cycle, to_pe, ...) |
There was a problem hiding this comment.
The docstring says the function returns a specific dictionary format but actually returns a tuple of (graph, reverse_graph, all_events). Update the docstring to accurately describe the return value.
Suggested change
| Returns a dict mapping (cycle, from_pe, direction, channel) -> (cycle, to_pe, ...) | |
| Returns: | |
| tuple: (graph, reverse_graph, all_events) | |
| - graph: dict mapping sink node (cycle, to_pe, direction, channel) to a list of (from_node, edge_info) | |
| - reverse_graph: dict mapping source node (cycle, from_pe, direction, channel) to a list of (to_node, edge_info) | |
| - all_events: list of all parsed event dictionaries from the log file |
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.
No description provided.