Open mkurc-ant opened 3 years ago
I think the bug is that pack pattern's (excluding the special case of carry chains) should only appear on "internal" nets inside a device. It should not be generated for the connections to the top level ports?
That is one thing which can probably be fixed by adding a port attribute that would allow/disallow annotating the "outward" connection with a pack pattern.
However, the second issue is that you assign these attributes not to wires but to nets. A net can span multiple cells making it impossible to explicitly specify a pack pattern on individual cell-to-cell connections. Here is an example:
module BLOCK(
input wire [3:0] lut_in,
output wire lut_out,
output wire ff1_out,
output wire ff2_out
);
wire w;
wire w1;
wire w2;
// LUT site
LUT lut (.I(lut_in), .O(w));
assign w1 = w;
assign w2 = w;
// FF sites
FF ff1 (.I(w1), .O(ff1_out));
FF ff2 (.I(w2), .O(ff2_out));
// Combinational LUT output
assign lut_out = w;
endmodule
Its a LUT that drives 2 FFs. The three wires w
, w1
and w2
form a single net that gets the (* PACK *)
annotation. When you specify different (* PACK *)
attributes on w1
and w2
you get a conflict.
We either need to come up with another way of specifying pack patterns or generate them automatically on all cell-to-cell connections as you have suggested in https://github.com/SymbiFlow/python-symbiflow-v2x/pull/75#discussion_r496894745. Although it is still unclear for me whether the latter approach would be always feasible.
It seems like the net should be decoded into (src, dst)
pairs and then auto-detect if a pack pattern is needed for each pair? A pack pattern is needed if neither src
nor dst
is a top level port?
@mithro There is one exception for that which is a carry-chain pack-pattern that actually has to include top-level port.
I'd like the auto-detection of pack-patterns to be more controlled hence I suggest that we do that only for nets that have the PACK
attribute (with no value) provided. Pack-pattern names would be auto-generated basing on cell names and would be specific for each (src, dst)
pair.
By default pack-pattern annotation would not include top-level ports unless a port has the PACK
attribute on it as well. Since carry-chain pack-patterns must have the same name in all affected pb_types the PACK
attribute on a port would take a value of that name.
There is one issue with the proposed cell-to-cell pack-pattern generation: It won't allow to propagate the pack pattern through different levels of pb_type hierarchy. The propagation would require parsing included XMLs as V2X includes XMLs for child cells.
@mkurc-ant -- v2x already separates out the carry-chain pack-patterns from the other pack-patterns. For a carry-chain you use the (* carry *)
attribute rather than the (* pack *)
attribute.
@mithro I've just realized that the problem is more complicated than that. Restricting (* pack *)
to only cell-to-cell connections would disallow specifying pack-patterns that spans multiple levels of pb_type hierarchy. In these cases top-level ports should actually be included.
Another thing is that automatic pack-pattern generation for (src, dst)
pairs of a net based on cell names will be always local to a specific pb_type. This will also prevent us from making pack-patterns that cross pb_type hierarchy.
An example case when a pack-pattern must include pb_types from different locations in the hierarchy is model of the QuickLogic EOS-S3 LOGIC
cell. In this file there are two pack patterns that should be specified on a connection between a flip-flop leaf pb_type and an output of its parent pb_type: https://github.com/QuickLogic-Corp/symbiflow-arch-defs/blob/c6fab2118bec32b2bf6824d1c1afa92fb277d70d/quicklogic/pp3/primitives/logic/q_frag_modes.sim.v#L21
These pack-pattern names are used throughout the whole LOGIC
cell hence automatic pack-pattern name generation is not an option for such a case.
GitHubFOSS architecture definitions of FPGA hardware useful for doing PnR device generation. - QuickLogic-Corp/symbiflow-arch-defs
@mithro I have an idea how to solve pack-patterns for nets with multiple sinks but it would require more work: In Yosys you can use the insbuf
pass which inserts $_BUF_
cells between connected wires which effectively makes them separate nets. V2X could process such a netlist and determine which individual wires have (* pack *)
attributes and to what cells they connect to (the latter is not possible now). That would allow to propagate the pack-pattern only to a relevant branch of a net.
Rather than putting the (* pack *)
on the wire, can we put the (* pack *)
on the port of the cell?
So Yosys accepts attribute on cell port connection but unfortunately it discards it. This gets parsed but the attribute content isn't stored anywhere:
// LUT site
LUT lut (.I(lut_in), (* PACK="lut_to_ff" *) .O(w));
Did you have a patch for adding surelog support for this type of thing?
I used surelog to get parameters from attributes. I have to check how hard would it be to get attributes from port connections.
Consider the following module definition:
The pack pattern is intended to be attached only to the
LUT
-FF
connection. But in the end it is also attached to theLUT
and the top-levellut_out
port connection.The problem is that by assigning the
(* PACK *)
attribute to a wire it is effectively assigned to the whole net.