Error due to flip flop with shape view on Xilinx platform

[jorolf]

Currently, if you try to use a flip flop with a shaped input, the conversion fails when using XilinxPlatform.
Other places in the code might be affected to, I haven't checked that.

To reproduce:

from amaranth import *
from amaranth.lib import wiring, cdc, data

class FooStruct(data.Struct):
    one: 1
    two: 2

class Foo(wiring.Component):

    input: wiring.In(FooStruct)
    output: wiring.Out(FooStruct)

    def elaborate(self, _platform):
        m = Module()

        m.domains.bar = ClockDomain(local=True)

        m.d.comb += [
            ClockSignal("bar").eq(1),
            ResetSignal("bar").eq(0),
        ]

        m.submodules[f"cdc_bar"] = cdc.FFSynchronizer(
            i = self.input,
            o = self.output,
            o_domain = "bar",
            init = 0,
        )

        return m

if __name__ == "__main__":
    from amaranth.back import verilog
    # Any implementation of XilinxPlatform
    from amaranth_boards.microzed_z020 import MicroZedZ020Platform

    verilog.convert(Foo(), name="foo", platform=MicroZedZ020Platform())

The error:

Traceback (most recent call last):
  File ".../venv/lib/python3.10/site-packages/amaranth/lib/data.py", line 865, in __getattr__
    item = self[name]
  File ".../venv/lib/python3.10/site-packages/amaranth/lib/data.py", line 833, in __getitem__
    field = self.__layout[key]
  File ".../venv/lib/python3.10/site-packages/amaranth/lib/data.py", line 344, in __getitem__
    return self._fields[key]
KeyError: 'attrs'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File ".../ff_shape.py", line 37, in <module>
    verilog.convert(Foo(), name="foo", platform=MicroZedZ020Platform())
  File ".../venv/lib/python3.10/site-packages/amaranth/back/verilog.py", line 60, in convert
    fragment = _ir.Fragment.get(elaboratable, platform)
  File ".../venv/lib/python3.10/site-packages/amaranth/hdl/_ir.py", line 63, in get
    new_obj = obj.elaborate(platform)
  File ".../venv/lib/python3.10/site-packages/amaranth/hdl/_dsl.py", line 687, in elaborate
    fragment.add_subfragment(Fragment.get(submodule, platform), name, src_loc=src_loc)
  File ".../venv/lib/python3.10/site-packages/amaranth/hdl/_ir.py", line 63, in get
    new_obj = obj.elaborate(platform)
  File ".../venv/lib/python3.10/site-packages/amaranth/lib/cdc.py", line 94, in elaborate
    return platform.get_ff_sync(self)
  File ".../venv/lib/python3.10/site-packages/amaranth/vendor/_xilinx.py", line 1243, in get_ff_sync
    flops[0].attrs["amaranth.vivado.false_path"] = "TRUE"
  File ".../venv/lib/python3.10/site-packages/amaranth/lib/data.py", line 867, in __getattr__
    raise AttributeError(
AttributeError: View with layout StructLayout({'one': 1, 'two': 2}) does not have a field 'attrs'; did you mean one of: 'one', 'two'?

I suspect this is because this line creates a signal with the shape:

amaranth/amaranth/vendor/_xilinx.py

Lines 1238 to 1241 in 590cba1

	flops = [Signal(ff_sync.i.shape(), name=f"stage{index}",
	init=ff_sync._init, reset_less=ff_sync._reset_less,
	attrs={"ASYNC_REG": "TRUE"})
	for index in range(ff_sync._stages)]

Which is then accessed here:

amaranth/amaranth/vendor/_xilinx.py

Line 1244 in 590cba1

flops[0].attrs["amaranth.vivado.false_path"] = "TRUE"

But because of the metaclass of Signal what actually gets created is a View:

amaranth/amaranth/hdl/_ast.py

Lines 1941 to 1946 in 590cba1

	class _SignalMeta(ABCMeta):
	def __call__(cls, shape=None, src_loc_at=0, **kwargs):
	signal = super().__call__(shape, **kwargs, src_loc_at=src_loc_at + 1)
	if isinstance(shape, ShapeCastable):
	return shape(signal)
	return signal

[whitequark]

Thanks, we completely missed this case.

[jorolf]

I fixed it for now by wrapping the parameters in Value.cast():

m.submodules[f"cdc_bar"] = cdc.FFSynchronizer(
    i = Value.cast(self.input),
    o = Value.cast(self.output),
    o_domain = "bar",
    init = 0,
)

Although I'm not sure if this is actually "correct"

[whitequark]

That workaround will work fine.