Skip to content

Commit

Permalink
hdl.rec: don't save casted shapes in Layout constructor
Browse files Browse the repository at this point in the history 
Don't save the result of Shape.cast in the Layout constructor.
This allows the original user-provided shape to be preserved,
while still checking its validity.

This fixes Enum decoder inference for Signals (amaranth-lang#393).
  • Loading branch information
@shawnanastasio
shawnanastasio committed Jun 2, 2020
1 parent 26a15b3 commit 7243fe2
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 40 deletions.
5 changes: 3 additions & 2 deletions nmigen/hdl/rec.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ def __init__(self, fields, *, src_loc_at=0):
.format(field))
if not isinstance(shape, Layout):
try:
shape = Shape.cast(shape, src_loc_at=1 + src_loc_at)
# Check provided shape by calling Shape.cast and checking for exception
Shape.cast(shape, src_loc_at=1 + src_loc_at)
except Exception as error:
raise TypeError("Field {!r} has invalid shape: should be castable to Shape "
"or a list of fields of a nested record"
Expand Down Expand Up @@ -134,7 +135,7 @@ def concat(a, b):
if isinstance(field_shape, Layout):
assert isinstance(field, Record) and field_shape == field.layout
else:
assert isinstance(field, Signal) and field_shape == field.shape()
assert isinstance(field, Signal) and Shape.cast(field_shape) == field.shape()
self.fields[field_name] = field
else:
if isinstance(field_shape, Layout):
Expand Down
27 changes: 16 additions & 11 deletions nmigen/test/test_hdl_rec.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ class UnsignedEnum(Enum):


class LayoutTestCase(FHDLTestCase):
def assertFieldEqual(self, field, expected):
(shape, dir) = field
shape = Shape.cast(shape)
self.assertEqual((shape, dir), expected)

def test_fields(self):
layout = Layout.cast([
("cyc", 1),
Expand All @@ -24,28 +29,28 @@ def test_fields(self):
])
])

self.assertEqual(layout["cyc"], ((1, False), DIR_NONE))
self.assertEqual(layout["data"], ((32, True), DIR_NONE))
self.assertEqual(layout["stb"], ((1, False), DIR_FANOUT))
self.assertEqual(layout["ack"], ((1, False), DIR_FANIN))
self.assertFieldEqual(layout["cyc"], ((1, False), DIR_NONE))
self.assertFieldEqual(layout["data"], ((32, True), DIR_NONE))
self.assertFieldEqual(layout["stb"], ((1, False), DIR_FANOUT))
self.assertFieldEqual(layout["ack"], ((1, False), DIR_FANIN))
sublayout = layout["info"][0]
self.assertEqual(layout["info"][1], DIR_NONE)
self.assertEqual(sublayout["a"], ((1, False), DIR_NONE))
self.assertEqual(sublayout["b"], ((1, False), DIR_NONE))
self.assertFieldEqual(sublayout["a"], ((1, False), DIR_NONE))
self.assertFieldEqual(sublayout["b"], ((1, False), DIR_NONE))

def test_enum_field(self):
layout = Layout.cast([
("enum", UnsignedEnum),
("enum_dir", UnsignedEnum, DIR_FANOUT),
])
self.assertEqual(layout["enum"], ((2, False), DIR_NONE))
self.assertEqual(layout["enum_dir"], ((2, False), DIR_FANOUT))
self.assertFieldEqual(layout["enum"], ((2, False), DIR_NONE))
self.assertFieldEqual(layout["enum_dir"], ((2, False), DIR_FANOUT))

def test_range_field(self):
layout = Layout.cast([
("range", range(0, 7)),
])
self.assertEqual(layout["range"], ((3, False), DIR_NONE))
self.assertFieldEqual(layout["range"], ((3, False), DIR_NONE))

def test_slice_tuple(self):
layout = Layout.cast([
Expand All @@ -60,9 +65,9 @@ def test_slice_tuple(self):
self.assertEqual(layout["a", "c"], expect)

def test_repr(self):
self.assertEqual(repr(Layout([("a", 1), ("b", signed(2))])),
self.assertEqual(repr(Layout([("a", unsigned(1)), ("b", signed(2))])),
"Layout([('a', unsigned(1)), ('b', signed(2))])")
self.assertEqual(repr(Layout([("a", 1), ("b", [("c", signed(3))])])),
self.assertEqual(repr(Layout([("a", unsigned(1)), ("b", [("c", signed(3))])])),
"Layout([('a', unsigned(1)), "
"('b', Layout([('c', signed(3))]))])")

Expand Down
61 changes: 34 additions & 27 deletions nmigen/test/test_lib_io.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,104 +4,111 @@
from ..back.pysim import *
from ..lib.io import *

class PinLayoutTestCase(FHDLTestCase):
def assertLayoutEqual(self, layout, expected):
casted_layout = {}
for name, (shape, dir) in layout.items():
casted_layout[name] = (Shape.cast(shape), dir)

class PinLayoutCombTestCase(FHDLTestCase):
self.assertEqual(casted_layout, expected)

class PinLayoutCombTestCase(PinLayoutTestCase):
def test_pin_layout_i(self):
layout_1 = pin_layout(1, dir="i")
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="i")
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i": ((2, False), DIR_NONE),
})

def test_pin_layout_o(self):
layout_1 = pin_layout(1, dir="o")
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="o")
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o": ((2, False), DIR_NONE),
})

def test_pin_layout_oe(self):
layout_1 = pin_layout(1, dir="oe")
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o": ((1, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="oe")
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o": ((2, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

def test_pin_layout_io(self):
layout_1 = pin_layout(1, dir="io")
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i": ((1, False), DIR_NONE),
"o": ((1, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="io")
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i": ((2, False), DIR_NONE),
"o": ((2, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})


class PinLayoutSDRTestCase(FHDLTestCase):
class PinLayoutSDRTestCase(PinLayoutTestCase):
def test_pin_layout_i(self):
layout_1 = pin_layout(1, dir="i", xdr=1)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i_clk": ((1, False), DIR_NONE),
"i": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="i", xdr=1)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i_clk": ((1, False), DIR_NONE),
"i": ((2, False), DIR_NONE),
})

def test_pin_layout_o(self):
layout_1 = pin_layout(1, dir="o", xdr=1)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o_clk": ((1, False), DIR_NONE),
"o": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="o", xdr=1)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o_clk": ((1, False), DIR_NONE),
"o": ((2, False), DIR_NONE),
})

def test_pin_layout_oe(self):
layout_1 = pin_layout(1, dir="oe", xdr=1)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o_clk": ((1, False), DIR_NONE),
"o": ((1, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="oe", xdr=1)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o_clk": ((1, False), DIR_NONE),
"o": ((2, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

def test_pin_layout_io(self):
layout_1 = pin_layout(1, dir="io", xdr=1)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i_clk": ((1, False), DIR_NONE),
"i": ((1, False), DIR_NONE),
"o_clk": ((1, False), DIR_NONE),
Expand All @@ -110,7 +117,7 @@ def test_pin_layout_io(self):
})

layout_2 = pin_layout(2, dir="io", xdr=1)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i_clk": ((1, False), DIR_NONE),
"i": ((2, False), DIR_NONE),
"o_clk": ((1, False), DIR_NONE),
Expand All @@ -119,48 +126,48 @@ def test_pin_layout_io(self):
})


class PinLayoutDDRTestCase(FHDLTestCase):
class PinLayoutDDRTestCase(PinLayoutTestCase):
def test_pin_layout_i(self):
layout_1 = pin_layout(1, dir="i", xdr=2)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i_clk": ((1, False), DIR_NONE),
"i0": ((1, False), DIR_NONE),
"i1": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="i", xdr=2)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i_clk": ((1, False), DIR_NONE),
"i0": ((2, False), DIR_NONE),
"i1": ((2, False), DIR_NONE),
})

def test_pin_layout_o(self):
layout_1 = pin_layout(1, dir="o", xdr=2)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o_clk": ((1, False), DIR_NONE),
"o0": ((1, False), DIR_NONE),
"o1": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="o", xdr=2)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o_clk": ((1, False), DIR_NONE),
"o0": ((2, False), DIR_NONE),
"o1": ((2, False), DIR_NONE),
})

def test_pin_layout_oe(self):
layout_1 = pin_layout(1, dir="oe", xdr=2)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"o_clk": ((1, False), DIR_NONE),
"o0": ((1, False), DIR_NONE),
"o1": ((1, False), DIR_NONE),
"oe": ((1, False), DIR_NONE),
})

layout_2 = pin_layout(2, dir="oe", xdr=2)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"o_clk": ((1, False), DIR_NONE),
"o0": ((2, False), DIR_NONE),
"o1": ((2, False), DIR_NONE),
Expand All @@ -169,7 +176,7 @@ def test_pin_layout_oe(self):

def test_pin_layout_io(self):
layout_1 = pin_layout(1, dir="io", xdr=2)
self.assertEqual(layout_1.fields, {
self.assertLayoutEqual(layout_1.fields, {
"i_clk": ((1, False), DIR_NONE),
"i0": ((1, False), DIR_NONE),
"i1": ((1, False), DIR_NONE),
Expand All @@ -180,7 +187,7 @@ def test_pin_layout_io(self):
})

layout_2 = pin_layout(2, dir="io", xdr=2)
self.assertEqual(layout_2.fields, {
self.assertLayoutEqual(layout_2.fields, {
"i_clk": ((1, False), DIR_NONE),
"i0": ((2, False), DIR_NONE),
"i1": ((2, False), DIR_NONE),
Expand Down

0 comments on commit 7243fe2

Please sign in to comment.