Skip to content

Ensure Emulation core executes each micro-op to completion in 1 cycle#417

Merged
FinnWilkinson merged 12 commits intodevfrom
atomic-emulation
Jul 9, 2024
Merged

Ensure Emulation core executes each micro-op to completion in 1 cycle#417
FinnWilkinson merged 12 commits intodevfrom
atomic-emulation

Conversation

@FinnWilkinson
Copy link
Copy Markdown
Contributor

The previous emulation core implementation did not ensure that load instructions completed in a single cycle. Given we restrict emulation cores to only use Flat emmory interfaces (i.e. no cycle delay to update or access memory) there is no need for each load instruction to take 2 cycles.

This PR also simplifies some of the logic in the tick() function of the emulation core.

@FinnWilkinson FinnWilkinson added the bug Something isn't working label Jun 19, 2024
@FinnWilkinson FinnWilkinson self-assigned this Jun 19, 2024
Copy link
Copy Markdown
Contributor

@ABenC377 ABenC377 left a comment

Choose a reason for hiding this comment

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

Looks good. Only minor comment

Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Copy link
Copy Markdown
Contributor

@dANW34V3R dANW34V3R left a comment

Choose a reason for hiding this comment

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

Minor comments. Looking good

Comment thread .jenkins/build_test_run.sh Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Copy link
Copy Markdown
Contributor

@jj16791 jj16791 left a comment

Choose a reason for hiding this comment

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

A few comments to do with more future-proofing and changed the emulation core to work on a per-macroOp basis rather than a per-microOp basis

Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc
Comment thread src/lib/models/emulation/Core.cc
jj16791
jj16791 previously approved these changes Jul 4, 2024
ABenC377
ABenC377 previously approved these changes Jul 4, 2024
JosephMoore25
JosephMoore25 previously approved these changes Jul 4, 2024
Copy link
Copy Markdown
Contributor

@dANW34V3R dANW34V3R left a comment

Choose a reason for hiding this comment

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

Stylistic comments

Comment thread src/lib/models/emulation/Core.cc Outdated
Comment thread src/lib/models/emulation/Core.cc Outdated
@FinnWilkinson FinnWilkinson dismissed stale reviews from JosephMoore25, ABenC377, and jj16791 via 43e4ca9 July 4, 2024 16:30
@FinnWilkinson
Copy link
Copy Markdown
Contributor Author

#rerun tests

@FinnWilkinson FinnWilkinson merged commit 9f4a1e0 into dev Jul 9, 2024
@FinnWilkinson FinnWilkinson deleted the atomic-emulation branch August 22, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants