Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a ReLU testbench/program and extends the emulator to support additional ops and updated exit behavior, intended to address reported functional issues.
Changes:
- Add ReLU testbench YAML program + DFG and a runnable Go testbench driver.
- Extend core emulator with
SELandICMP_SGE, restructure return handling with an “exit delay”, and improve an OOB store panic message. - Minor tweaks to histogram testbench output printing.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| test/testbench/relu/relu.yaml | Adds a ReLU instruction schedule/program for a 4x4 array. |
| test/testbench/relu/relu-dfg.yaml | Adds the corresponding DFG for the ReLU program. |
| test/testbench/relu/main.go | Adds a runnable ReLU testbench driver that loads the YAML and runs the sim. |
| test/testbench/histogram/main.go | Adjusts printing logic and adds commented import line. |
| core/emu.go | Adds SEL + ICMP_SGE, changes return/exit handling, and adds several debug prints. |
| core/builder.go | Threads a new shared exit-request timestamp pointer into core state. |
| config/config.go | Initializes and wires the shared exit-request timestamp for all cores. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| c.state = coreState{ | ||
| exit: b.exitAddr, | ||
| retVal: b.retValAddr, | ||
| SelectedBlock: nil, | ||
| PCInBlock: -1, | ||
| exit: b.exitAddr, | ||
| retVal: b.retValAddr, | ||
| requestExitTimestamp: b.exitReqAddr, | ||
| SelectedBlock: nil, |
There was a problem hiding this comment.
Build copies b.exitReqAddr into coreState.requestExitTimestamp, but exitReqAddr is optional and can be nil. Since runRet/RunInstructionGroup dereference this pointer, Build should ensure it is always non-nil (e.g., default to an internal float64 when nil) to avoid runtime panics for API consumers.
|
@n0thingNoob I've opened a new pull request, #37, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
How does this PR fix the phi_start bug when there are multiple phi_start in one core? |
n0thingNoob
left a comment
n0thingNoob
left a comment
There was a problem hiding this comment.
Can you resolve the conflict?
n0thingNoob
left a comment
n0thingNoob
left a comment
There was a problem hiding this comment.
LGTM, remember to fix the lint problem
No description provided.