RRNode side method no longer correct

[litghost]

Context

#1448 changed the rr graph build to only generate one IPIN/OPIN RR node even if the relevant cluster had pins on multiple sides of the tile. However when this change was made, the underlying RRNode method for getting the sides of a node was not changed, see:

vtr-verilog-to-routing/vpr/src/route/rr_graph_storage.h

Lines 189 to 195 in 37095c7

	e_side node_side(RRNodeId id) const {
	return get_node_side(
	vtr::array_view_id<RRNodeId, const t_rr_node_data>(
	node_storage_.data(), node_storage_.size()),
	id);
	}
	const char* node_side_string(RRNodeId id) const;

node_side still returns just one value, even if the underlying RR node is actually representing multiple sides.

Code like:

vtr-verilog-to-routing/vpr/src/route/check_rr_graph.cpp

Lines 565 to 576 in 37095c7

	static bool has_adjacent_channel(const t_rr_node& node, const DeviceGrid& grid) {
	VTR_ASSERT(node.type() == IPIN || node.type() == OPIN);
	if ((node.xlow() == 0 && node.side() != RIGHT) //left device edge connects only along block's right side
	|| (node.ylow() == int(grid.height() - 1) && node.side() != BOTTOM) //top device edge connects only along block's bottom side
	|| (node.xlow() == int(grid.width() - 1) && node.side() != LEFT) //right deivce edge connects only along block's left side
	|| (node.ylow() == 0 && node.side() != TOP) //bottom deivce edge connects only along block's top side
	) {
	return false;
	}
	return true; //All other blocks will be surrounded on all sides by channels
	}

no longer work when a node represents multiple sides.

FYI @tangxifan / @vaughnbetz

Possible Solution

Now that IPIN/OPIN nodes can represent pins on multiple sides of the tile, the relevant node methods need to be updated to account for this. Possible solutions:

• Change side() and node_side() to sides() and node_sides()
• Remove side() and node_side() and add is_on_side()

Etc

[tangxifan]

@litghost I noticed this issue when fixing the bug in routing resource graph. I have the same idea on changing the side() and node_side() method to sides() and node_sides(). But I am not sure what the is_on_side() will do? Does it record which side is used by the router?

[litghost]

The signature of is_on_side() would be bool RRNode::is_on_side(RRNode node, Side side), and returns true if the rr node connects to that side. This would be a scalar version of sides or node_sides.

[tangxifan]

@litghost Got it. Do you expect me to fix this on another pull request? Or you have a solution in the Symbiflow version?

[litghost]

@litghost Got it. Do you expect me to fix this on another pull request? Or you have a solution in the Symbiflow version?

We are have a workaround, but the current VPR data model in the upstream VPR code base is broken until this is fixed. I'm surprised check_rr_route didn't fail somewhere, because we both agree that node_side no longer works correctly. I suspect we've just gotten lucky so far.

[vaughnbetz]

Yup, we should fix. Depending on what the callers want, I am fine with coding only sides / node_sides, or also coding is_on_side if callers would also benefit from that.