matthijsr / til-vhdl

A prototype toolchain for demonstrating and exploring an intermediate representation for defining components using the Tydi interface specification.
Apache License 2.0
3 stars 1 forks source link

Emit a testbench based on assertions #11

Open matthijsr opened 2 years ago

matthijsr commented 2 years ago

After discussion: Being able to assert Streamlet behaviour as "Given input X, next handshake: output should be Y" was deemed useful.

However, since ports of Streamlets are not simple signals, such assertions are not quite as straightforward. A Stream will always result in multiple signals (data/valid/etc.), and can potentially be split up into multiple Physical Streams during Synthesis, if they carry a data type which contains or is itself a Stream.

As such, I propose that tests be able to access these resulting sub-signals/streams. E.g., for simple streams:

test simple_streams {
    // Create an instance of a streamlet
    subject a = streamlet1; // (Let's pretend this is a 2-bit adder)

    // arrange blocks everything else until it is complete, it can be used for resetting streamlets before testing them.
    arrange {
        reset a;
        wait 5 cycles; // A simple wait can ensure a proper reset according to the designer's intentions.
        !reset a;
    }

    // act sets the data, sets valid, then waits for ready (0->)1->0 before setting valid and data to 0.
    // act only takes sink ports. ("in" Forward, or "out" Reverse)
    act a.port_a = "01";
    act a.port_b = "01"; // Potential for some built-in functions for numbers, strings, and random generation in the future

    // assert is the inverse of act, in that it only takes source ports, and waits for valid 1
    // Rather than assigning signals, act performs a comparison.
    assert a.port_c = "010";
}

Note that all acts and assertions should be emitted as parallel processes per port. (Unless there's a use-case for tying them together?)

Things to consider:

More complex scenarios to follow.

matthijsr commented 2 years ago

Dimensionality

test dimensionality {
    // [...]

    // Using [[..],..] sets the necessary "last" signals for dimensionality.
    act a.port_a = [["1", "1"], ["0", "1"]];

    // On assertion, the last signal is instead verified to match expectations.
    assert a.port_b = ["11", "01"];
}
matthijsr commented 2 years ago

Element lanes and lane validity (Complexity)

Complexity:

test complexity {
    // [...]

    // Using '-' indicates that the strb/stai/endi signals should be set to indicate a partially invalid signal
    act a.port_a = "001-0";

    // Also works on assertion
    assert a.port_b = "--1--";
}

~~Something to consider: Maybe have a "wildcard" option for assert? E.g. $"*****" ('1', '1'); to indicate "I expect two '1's anywhere in this response", and $"****" ("11"); to indicate "I expect "11" in one continuous block in this response.~~

EDIT:

This isn't quite accurate to how throughput/element lanes work. https://abs-tudelft.github.io/tydi/specification/physical.html#last-signal-description

Note that adjacent lanes can carry values which cross multiple dimensions.

So a more accurate way of portraying lanes would be:

a.port_a = ([[["111", "111"], ["111",)("111", "111"],["111"]],)([["111"]]] - -);

Where each ( ) indicates a transfer, and the [ ] dimension brackets can bridge transfers.

a.port_a =
(
[ [ ["111",
     "111"],
    ["111",
)
(
    "111",
    "111"],
    ["111"]],
)
(
  [ ["111"]]]
-
-
);

~~But this is obviously very awkward to write (and potentially equally awkward to parse). So I need to come up with a better way of describing this.~~

SCRAPPED

After some further discussion, it was decided the IR will not communicate how the protocol is handled to this degree. It will be up to the back-end to decide how to distribute a data signal over multiple lanes, if individual lane validity/alignment is supported. (And for the time being, the VHDL back-end will simply align all elements, regardless of Complexity.)

matthijsr commented 2 years ago

Split streams

test multiple_streams {
    // [...]

    // When a port is split into multiple physical streams, use \ to address them individually.

    act a.port_a\x = "01";
    act a.port_a\y = "01";

    assert a.port_a\z = "010";
}

This will require some knowledge on the designer's part of how Tydi synthesizes Streams, but it's hard to think of a better solution. (edit: Which is not to say all of this couldn't be abstracted away by a front-end or some evaluated functions.)

NOTE:

Do account for the fact that separate physical streams can still be synchronous, see Sync/Desync: https://abs-tudelft.github.io/tydi/specification/logical.html#stream

As such, acting on streams completely in parallel, or acting on the same port in sequence without addressing synchronized streams, may not be supported.

We're not clear on how to handle this yet.

One option is that, like above, we don't actually expose these details in the IR, and simply show the physical streams as though they were still a field in a group.

Another option is that we add support for a "sync" block in addition to the default parallel act/asserts. E.g.:

sync {
  act a.port_a\a = "01";
  act a.port_a\b = "01";
}

Though either way, it will still allow someone to write incorrect signalling.

matthijsr commented 2 years ago

Sequences

Thinking on it, there are definitely use-cases for doing operations sequentially, like state machines and accumulators. So here's my idea:

test sequential {
    // [...]

    sequence {
        {
            act a.port_a = "01";
            assert a.port_b = "0001";
        },
        {
            act a.port_a = "00";
            assert a.port_b = "0001";
        },
        {
            act a.port_a = "01";
            assert a.port_b = "0010";
        },
    };
}

Where the act/assert processes are still parallel, but there's some flag which blocks them until each stage is complete before the next start. In effect, this just makes it so each stage is its own "test", they're just sharing the same "arrange" at the very beginning.

Something to consider: Maybe give all the stages names? That could result in better test feedback, e.g.:

test sequential {
    // [...]

    sequence {
        "add 1" {
            act a.port_a = "01";
            assert a.port_b = "0001";
        },
        "add nothing" {
            act a.port_a = "00";
            assert a.port_b = "0001";
        },
        "add 1 again" {
            act a.port_a = "01";
            assert a.port_b = "0010";
        },
    };
}

Which could then fail with Stage "add nothing": assert a.port_b expected "0001", actual "0000" Which is significantly more clear than if it had just been assert a.port_b expected "0001", actual "0000"

The IR will by default add a label when none is specified. (Just numbers.)

Parallel Sequences

Of note, there are situations where a designer might want to test sequences in parallel, or nest sequences.

For instance, a pipeline:

sequence "FIFO in" {
    "first" {
        act a.port_in = "001";
    },
    "second" {
        act a.port_in = "010";
    },
    "last" {
        act a.port_in = "011";
    },
};

sequence "FIFO out" {
    "first" {
        assert a.port_out = "001";
    },
    "second" {
        assert a.port_out = "010";
    },
    "last" {
        assert a.port_out = "011";
    },
};

In this case, it's not necessary for the assert on the output to finish for the next stage on the input to happen, or for the designer to know how many stages are in the pipeline.

Also note the ability to optionally name a sequence. This will simply prepend said name to the labels in some form ("sequence" -> "stage"?) for ease of use and clarity. Beyond that, using the same label multiple times is supported, as it's purely for test output and not for naming anything functional.

Note: As with regular acts and asserts, you cannot act or assert on the same port multiple times in the same parallel scope. So parallel sequence permanently exclude all ports in that sequence from being used in other sequences.

Also, you do not need to use sequences to test something in parallel to a sequence. Let's imagine something which concatenates a certain number of inputs:

sequence "FIFO in" {
    "first" {
        act a.port_in = "001";
    },
    "second" {
        act a.port_in = "010";
    },
    "last" {
        act a.port_in = "011";
    },
}

assert a.port_out = "001010011";

Nesting Sequences

There are also obviously cases where you may want to nest a sequence in another sequence, especially those sequences need to be in parallel:

sequence {
    "set mask and val" {
        sequence "mask" {
            {
                act a.port_a = "1";
            },
            {
                act a.port_a = "0";
            },
        };
        sequence "val" {
            {
                act a.port_b = "1";
            },
            {
                act a.port_b = "1";
            },
        };
    },
    "verify out" {
        assert a.port_c = "10";
    },
};

In this instance, the first two sequences happen in parallel, but both sequences need to complete for the next step in the enclosing sequence to start.

Note: When nesting sequences, which ports are acted on in parallel is obviously interpreted based on their scope. So the "verify out" stage could act on port_a and port_b, even though they were previously used in other sequences.

matthijsr commented 2 years ago

Element-manipulating types

Since the goal is to keep track of the original (element-manipulating) types #56 , it should be viable to address ports as such, respecting their proper signaling. (Addressing the fields of groups individually, and setting the tags of unions appropriately.)

What this looks like (and whether separate direct data addressing should still be possible) is TBD.

Current idea:

Null

// Null types use an explicit "Null" for act and assert
act a.a = Null;
assert a.b = Null;
// This is to accommodate instances where someone might
// have a multi-dimensional stream carrying Null data.
act a.a = [[Null, Null], [Null]];

Alternatively:

// Null types use a lack of anything at all
act a.a;
assert a.b;
// When addressing multiple dimensions, we simply
// count the commas to determine individual elements
act a.a = [[,], []];

This is possible to parse, but very awkward to read.

Bits

Bits use the same "1011" syntax as shown previously.

act a.a = "110";
assert a.b = "011";

Group

Groups use a simple { field: value, field: value } syntax.

It's worth noting that these fields may no longer align to the original type specification, as some of the individual fields on the logical type may have been synthesized into separate physical streams.

act a.a = { a: "10", b: Null, c: { a: "11", b: "00" } };
assert a.b = { a: "101", b: Null };

Union

Unions use a syntax like Groups, but only ever has a "union" and "tag" field.

act a.a = { union: "110", tag: "01" };
act a.b = { union: "11", tag: "10" };

The value entered into the union field will be right-aligned, with all other values left as they were. (You can also specify none, when Null or in the case described below.)

There's a particularly strange situation when accounting for Unions of Streams, as the enclosing Stream will still carry the tag data, but the other Streams are split into separate physical streams.

In this case, the union field will be empty, but the tag instead informs which physical stream is "active".

// In this example, the tag activates the Reverse stream.
act a.a = { tag: "11" };
assert a.a\d = "11011";

This is another instance where the "sync" block discussed in https://github.com/matthijsr/til-vhdl/issues/11#issuecomment-1050059188 might come in handy. When handling Forward Sync streams, for example.

sync {
  act a.a = { tag: "00" };
  act a.a\a = "11011";
};

Note to self: Need to check whether a stream split from a Union still has a "union" field (and maybe no tag?).

matthijsr commented 2 years ago

Musings:

It's possible to drop the act and assert keywords entirely, and instead infer whether something acts or asserts based on whether something is a sink or source.

This also (theoretically) obviates the need for a way to address physical streams manually, as a case like this:

a.port_inout = {
  in_field: "11",
  out_field: "11",
};

Where in_field and out_field are sink and source respectively, can simply be interpreted as separate acts and asserts on in_field and out_field. The back-end will then search whether something with that name exists as either a field on the port's base physical stream, or as a separate physical stream.

Furthermore, it allows the back-end to spawn synchronous or parallel processes depending on the enclosing Stream's specification.

Though this does assume that even Desync streams are ultimately coupled in some way. I.e., it's not possible to send more inputs to (or receive more outputs from) one stream than another.

I suppose that in the latter case, it would still be possible to support this behaviour by simply not addressing those fields:

sequence {
    {
        a.port = {
            in_field: "001",
        };
    },
    {
        a.port = {
            in_field: "001",
        };
    },
    {
        a.port = {
            in_field: "001",
        };
    },
    {
        a.port = {
            in_field: "001",
        };
    },
    {
        a.port = {
            out_field: "100",
        };
    },
};

Though this will get awkward when you want to act/assert on those logical fields (split streams) in parallel sequences. It's possible to still support it, but it means validation can no longer happen during the initial parse/pseudo-eval pass. (Needs to wait until synthesis on the query storage.)

matthijsr commented 2 years ago

Further notes on element lanes

It is possible, and in fact required, to keep track of element data activity at the IR level, even if it can be mostly abstracted away. This is because regardless of whether a Stream has 1 element lane, or multiple element lanes, a Stream is a sequence of elements.

Specifically, a Stream will keep track of dimensions and elements in a sequence. Taking: [[a, b], [c], [d]]

All the Stream actually cares about is:

[element, element (last of dim 0), element (last of dim 0), element (last of dim 0 and 1)]

If the Stream had 3 element lanes and aligned the elements, it would transfer this as:

At C >= 8 (per lane lasts): Transfer Content
1 [element, element (last of dim 0), element (last of dim 0)]
2 [element (last of dim 0 and 1), inactive, inactive]
At C < 8: Transfer Content
1 [element, element, inactive] (last of dim 0)
2 [element, inactive, inactive] (last of dim 0)
3 [element, inactive, inactive] (last of dim 0 and 1)

So we need to be able to set element lanes as inactive regardless. Which also means it should be trivial to set individual element transfers as “inactive data”, even without knowing what the number of element lanes is. I.e., rather than explicitly specifying those transfers (with some kind of bracket), we can just insert “inactive” markers inside any sequence, e.g.: The original, but explicit: [[a, b], [c], [d]], -, -

Arbitrary inactivity: [[a, -, b], -, [-, c], [d, -], -]

Which becomes: Transfer Content
1 [element, inactive, element (last of dim 0)]
2 [inactive, inactive, element (last of dim 0)]
3 [element, inactive (last of dim 0), inactive (last of dim 1)]

Note that “last” (dimension) validity is independent from data activity: https://abs-tudelft.github.io/tydi/specification/physical.html#last-signal-description