Stricter connections

Summary

Make it possible for lib.wiring.connect() to reject connections between ports with shapes that despite having equal widths are incompatible.

Motivation

The primary motivating use case is to be able to prevent connecting stream interfaces with incompatible payload shapes.

Guide-level explanation

lib.stream defines a basic stream interface with ready, valid and payload members. Additional metadata signals can be added by making the payload shape an aggregate. This has the advantage that such streams can be passed through standard components like FIFOs without them having to know or care about these signals.

Consider how we can make a byte-wide packetized stream by defining the payload shape as StructLayout({"data": 8, "last": 1}). The data field contains the data byte and the last field is a flag indicating that the current byte is the last in the packet.

This is not the only common way to make a packetized stream. Instead of (or in addition to) the last flag, the payload could have a first flag, indicating that the current byte is the first in the packet. Both are valid options with different semantics for different usecases.

The problem is that lib.wiring.connect() currently only checks that port members have matching widths, so a connection between a stream interface with last semantics and a stream interface with first semantics will not be rejected, despite being incompatible.

Currently, lib.data.View.eq() does no checks on the passed value and immediately forwards to self.as_value().eq(), making this legal:

>>> a = Signal(StructLayout({"data": 8, "last": 1}))
>>> b = Signal(StructLayout({"data": 8, "first": 1}))
>>> a.eq(b)
(eq (sig a) (sig b))

This RFC proposes adding a check to View.eq() that would reject direct assignments from another aggregate data structure with a non-identical layout. If such an assignment is desired, the other aggregate data structure can be explicitly passed through Value.cast() first.

Currently lib.wiring.connect() passes every signal through Value.cast() before assigning them. This results in a ValueCastable's .eq() not being called, and thereby bypassing the check proposed above.

This RFC proposes removing the Value.cast() so a ValueCastable's .eq() will be called directly.

This RFC proposes updating lib.enum.EnumView in the same manner, for the same reason, as lib.data.View.

Reference-level explanation

Modify lib.wiring.connect() to not pass port members through Value.cast(), so that a ValueCastable's .eq() will be called, allowing it to perform compatibility checks.

  • If a ValueCastable doesn't define .eq(), reject the assignment.

Modify lib.data.View.eq(other) to add the following checks:

  • If other is a ValueCastable, do Layout.cast(other.shape())
    • If a valid Layout is returned, reject the assignment if it doesn't match self.shape().
    • If Layout.cast() raises, reject the assignment.
  • Otherwise, proceed as normal.

Modify lib.enum.EnumView.eq(other) to add the following checks:

  • If other is a ValueCastable, reject the assignment if other.shape() doesn't match self.shape().
  • Otherwise, proceed as normal.

Rejected assignments are a warning in Amaranth 0.6 and becomes a hard error in Amaranth 0.7.

Drawbacks

  • Increased language complexity.

  • This will add an implied requirement for a ValueCastable to implement .eq() to be usable with lib.wiring. Currently a ValueCastable is not required to implement .eq() at all.

    • We could fall back to Value.cast().eq() when .eq() is not defined.

Rationale and alternatives

  • Signals like first and last could be added as separate ports in the stream signature, preventing incompatible connections.
    • This was already discussed in RFC 61 and decided against.
  • An explicit hook for checking compatibility between interfaces could be added, instead of relying on .eq().
    • This RFC proposes picking the low-hanging fruits first, making use of already existing mechanisms. If this is not enough, another RFC can propose an explicit hook later.

Prior art

RFC 61 already discussed the need to eventually make lib.wiring.connect() stricter to prevent the connection of stream interfaces with incompatible payloads.

Unresolved questions

None.

Future possibilities

  • A hook can still be added to allow a signature/interface to perform an explicit compatibility check, for cases where signatures have identical members but still have metadata indicating they are incompatible.
    • This hook could also allow overriding the existing checks, allowing connections where interfaces are compatible despite differences in members.