RR Graph - Cut interposer crossing wires #3342
Conversation
github-actions
bot
added
VPR
AmirhosseinPoolad
force-pushed
the
rr_graph_interposer_cut
branch
from
95dee19 to
9b6566b
Compare
AmirhosseinPoolad
changed the title
AlexandreSinger
left a comment
AlexandreSinger
left a comment
There was a problem hiding this comment.
Looks good. Some comments.
AmirhosseinPoolad
force-pushed
the
rr_graph_interposer_cut
branch
from
d4485f3 to
f007bde
Compare
| RRNodeId src_node = rr_graph.edge_src_node(edge_id); | ||
| RRNodeId sink_node = rr_graph.edge_sink_node(edge_id); | ||
|
|
||
| // TODO: ignoring ChanZ nodes for now |
There was a problem hiding this comment.
Can you elaborate a bit more on why chanz is ignored? My guess is that since this code is focused on the 2.5D architecture, the assumption is that chanz is not stretched across the 2D plane but along the Z-axis, so the interposer line wouldn’t apply to it. If that’s the case, I’d also add an ASSERT_DEBUG here to ensure that x_low == x_high and y_low == y_high.
There was a problem hiding this comment.
I haven't thought about 3D architectures that also have interposer cut lines so I'm honestly not sure what should happen to edges to connect to/from chanz nodes. I think this needs further discussions about what it even means to have an architecture like this.
There was a problem hiding this comment.
I'd update the comment to say that we don't allow interposers in the Z dimension (z is for die-stacking, and an interposer in a die stacking dimension is not currently manufacturable).
There was a problem hiding this comment.
It's actually a bit more concerning. We could, for example, have an L16 wire that crosses an interposer cut line, gets to a 3D switch block on the other side and connects to a chan z node. The current version of the code does not remove that edge while ideally it should be removed.
I think this should be addressed in a follow up PR.
This commit makes the bit mask much more obvious that it should be one-hot encoded.
AmirhosseinPoolad
force-pushed
the
rr_graph_interposer_cut
branch
from
f007bde to
e9876b3
Compare
| } | ||
| } | ||
|
|
||
| void update_interposer_crossing_nodes_in_spatial_lookup_and_rr_graph_storage(const RRGraphView& rr_graph, const DeviceGrid& grid, RRGraphBuilder& rr_graph_builder, const std::vector<std::pair<RRNodeId, int>>& sg_node_indices) { |
There was a problem hiding this comment.
the line is too long
the function name is too long
github-actions
bot
added
the
libarchfpga
AmirhosseinPoolad
force-pushed
the
rr_graph_interposer_cut
branch
from
88f8e82 to
b57b08f
Compare
vaughnbetz
left a comment
vaughnbetz
left a comment
There was a problem hiding this comment.
Looks good; some commenting upgrades and a few changes suggested.
This PR adds the last piece of functionality for modeling 2.5D architectures in upstream VPR: Actually cutting down the wires and connections that cross between the dice.
I had an issue with code duplication and there are still some functions that look very similar but do different things. I tried my best to keep the duplication to the lowest level functions and have more general higher level functions that wrap them but there's still some remaining. If you have an idea to do this in a better way please let me know.