Skip to content

Commit

Permalink
Fix fetch resume (#681)
Browse files Browse the repository at this point in the history
  • Loading branch information
piotro888 authored May 12, 2024
1 parent 83d706a commit 255abce
Showing 10 changed files with 93 additions and 29 deletions.
2 changes: 1 addition & 1 deletion coreblocks/backend/retirement.py
Original file line number Diff line number Diff line change
@@ -210,7 +210,7 @@ def flush_instr(rob_entry):
resume_pc = Mux(continue_pc_override, continue_pc, handler_pc)
m.d.sync += continue_pc_override.eq(0)

self.fetch_continue(m, pc=resume_pc, resume_from_exception=1)
self.fetch_continue(m, pc=resume_pc)

# Release pending trap state - allow accepting new reports
self.exception_cause_clear(m)
6 changes: 3 additions & 3 deletions coreblocks/core.py
Original file line number Diff line number Diff line change
@@ -118,10 +118,10 @@ def elaborate(self, platform):

m.submodules.exception_cause_register = self.exception_cause_register

fetch_resume, fetch_resume_unifiers = self.connections.get_dependency(FetchResumeKey())
fetch_resume_fb, fetch_resume_unifiers = self.connections.get_dependency(FetchResumeKey())
m.submodules.fetch_resume_unifiers = ModuleConnector(**fetch_resume_unifiers)

m.submodules.fetch_resume_connector = ConnectTrans(fetch_resume, self.frontend.fetch.resume)
m.submodules.fetch_resume_connector = ConnectTrans(fetch_resume_fb, self.frontend.fetch.resume_from_unsafe)

m.submodules.announcement = self.announcement
m.submodules.func_blocks_unifier = self.func_blocks_unifier
@@ -136,7 +136,7 @@ def elaborate(self, platform):
exception_cause_get=self.exception_cause_register.get,
exception_cause_clear=self.exception_cause_register.clear,
frat_rename=frat.rename,
fetch_continue=self.frontend.fetch.resume,
fetch_continue=self.frontend.fetch.resume_from_exception,
instr_decrement=self.frontend.core_counter.decrement,
trap_entry=self.interrupt_controller.entry,
)
79 changes: 69 additions & 10 deletions coreblocks/frontend/fetch/fetch.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
from amaranth import *
from amaranth.lib.data import ArrayLayout
from amaranth.lib.coding import PriorityEncoder
from coreblocks.interface.keys import FetchResumeKey
from transactron.lib import BasicFifo, Semaphore, ConnectTrans, logging, Pipe
from transactron.lib.metrics import *
from transactron.lib.simultaneous import condition
from transactron.utils import MethodLayout, popcount, assign
from transactron.utils.dependencies import DependencyContext
from transactron.utils.transactron_helpers import from_method_layout, make_layout
from transactron import *

@@ -51,12 +53,9 @@ def __init__(self, gen_params: GenParams, icache: CacheInterface, cont: Method)

self.layouts = self.gen_params.get(FetchLayouts)

self.resume = Method(i=self.layouts.resume)
self.resume_from_unsafe = Method(i=self.layouts.resume)
self.resume_from_exception = Method(i=self.layouts.resume)
self.stall_exception = Method()
# Fetch can be resumed to unstall from 'unsafe' instructions, and stalled because
# of exception report, both can happen at any time during normal excecution.
# ExceptionCauseRegister uses separate Transaction for it, so performace is not affected.
self.stall_exception.add_conflict(self.resume, Priority.LEFT)

self.perf_fetch_utilization = TaggedCounter(
"frontend.fetch.fetch_block_util",
@@ -358,21 +357,81 @@ def flush():
with m.If(flush_now):
m.d.sync += flushing_counter.eq(req_counter.count_next)

@def_method(m, self.resume, ready=(stalled & (flushing_counter == 0)))
def _(pc: Value, resume_from_exception: Value):
log.info(m, True, "Resuming new_pc=0x{:x} from exception={}", pc, resume_from_exception)
@def_method(m, self.resume_from_unsafe, ready=(stalled & (flushing_counter == 0)))
def _(pc: Value):
log.info(m, ~stalled_exception, "Resuming from unsafe instruction new_pc=0x{:x}", pc)
m.d.sync += current_pc.eq(pc)
# If core is stalled because of exception, effect of this call will be ignored, as
# `stalled_exception` is not changed
m.d.sync += stalled_unsafe.eq(0)

@def_method(m, self.resume_from_exception, ready=(stalled_exception & (flushing_counter == 0)))
def _(pc: Value):
log.info(m, True, "Resuming from exception new_pc=0x{:x}", pc)
# Resume from exception has implicit priority to resume from unsafe instructions call.
# Both could happen at the same time due to resume methods being blocked.
# `resume_from_unsafe` will never overwrite `resume_from_exception` event, because there is at most one
# unsafe instruction in the core that will call resume_from_unsafe before or at the same time as
# `resume_from_exception`.
# `current_pc` is set to correct entry at a complete unstall due to method declaration order
# See https://github.com/kuznia-rdzeni/coreblocks/pull/654#issuecomment-2057478960
m.d.sync += current_pc.eq(pc)
m.d.sync += stalled_unsafe.eq(0)
with m.If(resume_from_exception):
m.d.sync += stalled_exception.eq(0)
m.d.sync += stalled_exception.eq(0)

# Fetch can be resumed to unstall from 'unsafe' instructions, and stalled because
# of exception report, both can happen at any time during normal excecution.
# In case of simultaneous call, fetch will be correctly stalled, becasue separate signal is used
@def_method(m, self.stall_exception)
def _():
log.info(m, True, "Stalling the fetch unit because of an exception")
serializer.clean(m)
m.d.sync += stalled_exception.eq(1)
flush()

# Fetch resume verification
if self.gen_params.extra_verification:
expect_unstall_unsafe = Signal()
prev_stalled_unsafe = Signal()
unifier_ready = DependencyContext.get().get_dependency(FetchResumeKey())[0].ready
m.d.sync += prev_stalled_unsafe.eq(stalled_unsafe)
with m.FSM("running"):
with m.State("running"):
log.error(m, stalled_exception | prev_stalled_unsafe, "fetch was expected to be running")
log.error(
m,
unifier_ready,
"resume_from_unsafe unifier is ready before stall",
)
with m.If(stalled_unsafe):
m.next = "stalled_unsafe"
with m.If(self.stall_exception.run):
m.next = "stalled_exception"
with m.State("stalled_unsafe"):
m.d.sync += expect_unstall_unsafe.eq(1)
with m.If(self.resume_from_unsafe.run):
m.d.sync += expect_unstall_unsafe.eq(0)
m.d.sync += prev_stalled_unsafe.eq(0) # it is fine to be stalled now
m.next = "running"
with m.If(self.stall_exception.run):
m.next = "stalled_exception"
log.error(
m,
self.resume_from_exception.run & ~self.stall_exception.run,
"unexpected resume_from_exception",
)
with m.State("stalled_exception"):
with m.If(self.resume_from_unsafe.run):
log.error(m, ~expect_unstall_unsafe, "unexpected resume_from_unsafe")
m.d.sync += expect_unstall_unsafe.eq(0)
with m.If(self.resume_from_exception.run):
# unstall_form_unsafe may be skipped if excpetion was reported on unsafe instruction,
# invalid cases are verified by readiness check in running state
m.d.sync += expect_unstall_unsafe.eq(0)
m.d.sync += prev_stalled_unsafe.eq(0) # it is fine to be stalled now
with m.If(~self.stall_exception.run):
m.next = "running"

return m


7 changes: 3 additions & 4 deletions coreblocks/func_blocks/csr/csr.py
Original file line number Diff line number Diff line change
@@ -241,11 +241,10 @@ def _():

@def_method(m, self.fetch_resume, accepted)
def _():
# This call will always execute, because there is at most one usafe instruction in the core, and it can be
# stored in unifer's Forwarder unitl resume becomes ready.
# CSR instructions are never compressed, PC+4 is always next instruction
return {
"pc": instr.pc + self.gen_params.isa.ilen_bytes,
"resume_from_exception": False,
}
return {"pc": instr.pc + self.gen_params.isa.ilen_bytes}

# Generate precommitting signal from precommit
with Transaction().body(m):
2 changes: 1 addition & 1 deletion coreblocks/func_blocks/fu/priv.py
Original file line number Diff line number Diff line change
@@ -139,7 +139,7 @@ def _():
with m.Else():
log.info(m, True, "Unstalling fetch from the priv unit new_pc=0x{:x}", ret_pc)
# Unstall the fetch
self.fetch_resume_fifo.write(m, pc=ret_pc, resume_from_exception=0)
self.fetch_resume_fifo.write(m, pc=ret_pc)

return {
"rob_id": instr_rob,
2 changes: 1 addition & 1 deletion coreblocks/interface/layouts.py
Original file line number Diff line number Diff line change
@@ -455,7 +455,7 @@ def __init__(self, gen_params: GenParams):
fields.predicted_taken,
)

self.resume = make_layout(("pc", gen_params.isa.xlen), ("resume_from_exception", 1))
self.resume = make_layout(fields.pc)

self.predecoded_instr = make_layout(fields.cfi_type, ("cfi_offset", signed(21)), ("unsafe", 1))

4 changes: 4 additions & 0 deletions coreblocks/params/configurations.py
Original file line number Diff line number Diff line change
@@ -72,6 +72,8 @@ class _CoreConfigurationDataClass:
Log of the size of the fetch block (in bytes).
allow_partial_extensions: bool
Allow partial support of extensions.
extra_verification: bool
Enables generation of additional hardware checks (asserts via logging system). Defaults to True.
_implied_extensions: Extenstion
Bit flag specifing enabled extenstions that are not specified by func_units_config. Used in internal tests.
pma : list[PMARegion]
@@ -105,6 +107,8 @@ def __post_init__(self):

allow_partial_extensions: bool = False

extra_verification: bool = True

_implied_extensions: Extension = Extension(0)

pma: list[PMARegion] = field(default_factory=list)
2 changes: 2 additions & 0 deletions coreblocks/params/genparams.py
Original file line number Diff line number Diff line change
@@ -76,4 +76,6 @@ def __init__(self, cfg: CoreConfiguration):
self.fetch_width = 2**cfg.fetch_block_bytes_log // self.min_instr_width_bytes
self.fetch_width_log = exact_log2(self.fetch_width)

self.extra_verification = cfg.extra_verification

self._toolchain_isa_str = gen_isa_string(extensions, cfg.xlen, skip_internal=True)
16 changes: 8 additions & 8 deletions test/frontend/test_fetch.py
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@

from amaranth import Elaboratable, Module
from amaranth.sim import Passive
from coreblocks.interface.keys import FetchResumeKey

from transactron.core import Method
from transactron.lib import AdapterTrans, Adapter, BasicFifo
@@ -19,6 +20,7 @@
from coreblocks.params import *
from coreblocks.params.configurations import test_core_config
from coreblocks.interface.layouts import ICacheLayouts, FetchLayouts
from transactron.utils.dependencies import DependencyContext


class MockedICache(Elaboratable, CacheInterface):
@@ -69,13 +71,11 @@ def setup(self, configure_dependency_context):
fifo = BasicFifo(self.gen_params.get(FetchLayouts).raw_instr, depth=2)
self.io_out = TestbenchIO(AdapterTrans(fifo.read))
self.clean_fifo = TestbenchIO(AdapterTrans(fifo.clear))
fetch = FetchUnit(self.gen_params, self.icache, fifo.write)
self.fetch_resume = TestbenchIO(AdapterTrans(fetch.resume))
self.fetch_stall_exception = TestbenchIO(AdapterTrans(fetch.stall_exception))
self.fetch_resume_mock = TestbenchIO(Adapter())
DependencyContext.get().add_dependency(FetchResumeKey(), self.fetch_resume_mock.adapter.iface)
self.fetch = SimpleTestCircuit(FetchUnit(self.gen_params, self.icache, fifo.write))

self.m = ModuleConnector(
self.icache, fifo, self.io_out, self.clean_fifo, fetch, self.fetch_resume, self.fetch_stall_exception
)
self.m = ModuleConnector(self.icache, fifo, self.io_out, self.clean_fifo, self.fetch)

self.instr_queue = deque()
self.mem = {}
@@ -188,7 +188,7 @@ def fetch_out_check(self):

if (instr["jumps"] and (instr["branch_taken"] != v["predicted_taken"])) or access_fault:
yield from self.random_wait(5)
yield from self.fetch_stall_exception.call()
yield from self.fetch.stall_exception.call()
yield from self.random_wait(5)

# Empty the pipeline
@@ -203,7 +203,7 @@ def fetch_out_check(self):
) + self.gen_params.fetch_block_bytes

# Resume the fetch unit
while (yield from self.fetch_resume.call_try(pc=resume_pc, resume_from_exception=1)) is None:
while (yield from self.fetch.resume_from_exception.call_try(pc=resume_pc)) is None:
pass

def run_sim(self):
2 changes: 1 addition & 1 deletion transactron/core/method.py
Original file line number Diff line number Diff line change
@@ -160,7 +160,7 @@ def proxy(self, m: "TModule", method: "Method"):
Method for which this method is a proxy for.
"""

@def_method(m, self)
@def_method(m, self, ready=method.ready)
def _(arg):
return method(m, arg)

0 comments on commit 255abce

Please sign in to comment.