Skip to content

Comments

Squash reported bugs#36

Open
Jackcuii wants to merge 8 commits intomainfrom
fix-bugs
Open

Squash reported bugs#36
Jackcuii wants to merge 8 commits intomainfrom
fix-bugs

Conversation

@Jackcuii
Copy link
Collaborator

No description provided.

@Jackcuii Jackcuii requested a review from n0thingNoob January 28, 2026 21:50
@Jackcuii Jackcuii changed the title Squash bugs Squash reported bugs Jan 28, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 SEL and ICMP_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.

Comment on lines 46 to +56
c.state = coreState{
exit: b.exitAddr,
retVal: b.retValAddr,
SelectedBlock: nil,
PCInBlock: -1,
exit: b.exitAddr,
retVal: b.retValAddr,
requestExitTimestamp: b.exitReqAddr,
SelectedBlock: nil,
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI commented Jan 28, 2026

@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.

n0thingNoob and others added 2 commits January 28, 2026 17:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@n0thingNoob
Copy link
Contributor

n0thingNoob commented Jan 28, 2026

How does this PR fix the phi_start bug when there are multiple phi_start in one core?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants