From a7fbff94d84fbfb08f63d776b9329aa770bc5216 Mon Sep 17 00:00:00 2001 From: whitequark Date: Mon, 8 Jul 2019 10:26:49 +0000 Subject: [PATCH] hdl.{ast,cd,dsl,xfrm}: reject inappropriately used comb domain. Fixes #125. --- nmigen/hdl/ast.py | 4 ++++ nmigen/hdl/cd.py | 2 ++ nmigen/hdl/dsl.py | 2 ++ nmigen/hdl/xfrm.py | 5 +++++ nmigen/test/test_hdl_ast.py | 10 ++++++++++ nmigen/test/test_hdl_cd.py | 5 +++++ nmigen/test/test_hdl_dsl.py | 7 +++++++ nmigen/test/test_hdl_xfrm.py | 10 ++++++++++ 8 files changed, 45 insertions(+) diff --git a/nmigen/hdl/ast.py b/nmigen/hdl/ast.py index bf09077..949a825 100644 --- a/nmigen/hdl/ast.py +++ b/nmigen/hdl/ast.py @@ -693,6 +693,8 @@ class ClockSignal(Value): super().__init__(src_loc_at=src_loc_at) if not isinstance(domain, str): raise TypeError("Clock domain name must be a string, not '{!r}'".format(domain)) + if domain == "comb": + raise ValueError("Domain '{}' does not have a clock".format(domain)) self.domain = domain def shape(self): @@ -727,6 +729,8 @@ class ResetSignal(Value): super().__init__(src_loc_at=src_loc_at) if not isinstance(domain, str): raise TypeError("Clock domain name must be a string, not '{!r}'".format(domain)) + if domain == "comb": + raise ValueError("Domain '{}' does not have a reset".format(domain)) self.domain = domain self.allow_reset_less = allow_reset_less diff --git a/nmigen/hdl/cd.py b/nmigen/hdl/cd.py index 4d2a33b..06f8d79 100644 --- a/nmigen/hdl/cd.py +++ b/nmigen/hdl/cd.py @@ -50,6 +50,8 @@ class ClockDomain: raise ValueError("Clock domain name must be specified explicitly") if name.startswith("cd_"): name = name[3:] + if name == "comb": + raise ValueError("Domain '{}' may not be clocked".format(name)) self.name = name self.clk = Signal(name=self._name_for(name, "clk"), src_loc_at=1) diff --git a/nmigen/hdl/dsl.py b/nmigen/hdl/dsl.py index bd6399a..f154ed3 100644 --- a/nmigen/hdl/dsl.py +++ b/nmigen/hdl/dsl.py @@ -261,6 +261,8 @@ class Module(_ModuleBuilderRoot, Elaboratable): @contextmanager def FSM(self, reset=None, domain="sync", name="fsm"): self._check_context("FSM", context=None) + if domain == "comb": + raise ValueError("FSM may not be driven by the '{}' domain".format(domain)) fsm_data = self._set_ctrl("FSM", { "name": name, "signal": Signal(name="{}_state".format(name), src_loc_at=2), diff --git a/nmigen/hdl/xfrm.py b/nmigen/hdl/xfrm.py index 9448c0c..4340676 100644 --- a/nmigen/hdl/xfrm.py +++ b/nmigen/hdl/xfrm.py @@ -326,6 +326,11 @@ class DomainRenamer(FragmentTransformer, ValueTransformer, StatementTransformer) def __init__(self, domain_map): if isinstance(domain_map, str): domain_map = {"sync": domain_map} + for src, dst in domain_map.items(): + if src == "comb": + raise ValueError("Domain '{}' may not be renamed".format(src)) + if dst == "comb": + raise ValueError("Domain '{}' may not be renamed to '{}'".format(src, dst)) self.domain_map = OrderedDict(domain_map) def on_ClockSignal(self, value): diff --git a/nmigen/test/test_hdl_ast.py b/nmigen/test/test_hdl_ast.py index 6ef1193..54824b1 100644 --- a/nmigen/test/test_hdl_ast.py +++ b/nmigen/test/test_hdl_ast.py @@ -515,6 +515,11 @@ class ClockSignalTestCase(FHDLTestCase): s1 = ClockSignal() self.assertEqual(repr(s1), "(clk sync)") + def test_wrong_name_comb(self): + with self.assertRaises(ValueError, + msg="Domain 'comb' does not have a clock"): + ClockSignal("comb") + class ResetSignalTestCase(FHDLTestCase): def test_domain(self): @@ -534,6 +539,11 @@ class ResetSignalTestCase(FHDLTestCase): s1 = ResetSignal() self.assertEqual(repr(s1), "(rst sync)") + def test_wrong_name_comb(self): + with self.assertRaises(ValueError, + msg="Domain 'comb' does not have a reset"): + ResetSignal("comb") + class MockUserValue(UserValue): def __init__(self, lowered): diff --git a/nmigen/test/test_hdl_cd.py b/nmigen/test/test_hdl_cd.py index 43619f8..23c4637 100644 --- a/nmigen/test/test_hdl_cd.py +++ b/nmigen/test/test_hdl_cd.py @@ -55,3 +55,8 @@ class ClockDomainTestCase(FHDLTestCase): sync.rename("pix") self.assertEqual(sync.name, "pix") self.assertEqual(sync.clk.name, "pix_clk") + + def test_wrong_name_comb(self): + with self.assertRaises(ValueError, + msg="Domain 'comb' may not be clocked"): + comb = ClockDomain() diff --git a/nmigen/test/test_hdl_dsl.py b/nmigen/test/test_hdl_dsl.py index 43f83d2..9af7038 100644 --- a/nmigen/test/test_hdl_dsl.py +++ b/nmigen/test/test_hdl_dsl.py @@ -458,6 +458,13 @@ class DSLTestCase(FHDLTestCase): () """) + def test_FSM_wrong_domain(self): + m = Module() + with self.assertRaises(ValueError, + msg="FSM may not be driven by the 'comb' domain"): + with m.FSM(domain="comb"): + pass + def test_FSM_wrong_redefined(self): m = Module() with m.FSM(): diff --git a/nmigen/test/test_hdl_xfrm.py b/nmigen/test/test_hdl_xfrm.py index 3892299..fcf77c1 100644 --- a/nmigen/test/test_hdl_xfrm.py +++ b/nmigen/test/test_hdl_xfrm.py @@ -88,6 +88,16 @@ class DomainRenamerTestCase(FHDLTestCase): "pix": cd_pix, }) + def test_rename_wrong_to_comb(self): + with self.assertRaises(ValueError, + msg="Domain 'sync' may not be renamed to 'comb'"): + DomainRenamer("comb") + + def test_rename_wrong_from_comb(self): + with self.assertRaises(ValueError, + msg="Domain 'comb' may not be renamed"): + DomainRenamer({"comb": "sync"}) + class DomainLowererTestCase(FHDLTestCase): def setUp(self): -- 2.30.2