Skip to content

Commit

Permalink
Further improve handling of ports
Browse files Browse the repository at this point in the history 
- Distinguish register output ports in command-line reporting. This is
  a special case where a register (marked REG_SRC or REG_DST) is also a
  port.
- Limit port start and finish points to be the top-most instances in the
  hierarchy (eg i_data rather than <module>.i_data). This effectively
  removes duplicate port variables and reduces the number of fanin/out
  paths reported, which makes sense from a user point of view.
  • Loading branch information
Jamie Hanlon committed Feb 20, 2022
1 parent 4fa3c36 commit 9dedf78
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 107 deletions.
17 changes: 9 additions & 8 deletions docs/index.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ and view a list of named objects in the FSM example module:
➜ netlist-paths --compile examples/fsm.sv --dump-names
Name Type DType Width Direction Location
-------------- ---- ------------ ----- --------- ------------------
i_clk PORT logic 1 INPUT examples/fsm.sv:3
i_finish PORT logic 1 INPUT examples/fsm.sv:6
i_rst PORT logic 1 INPUT examples/fsm.sv:4
i_start PORT logic 1 INPUT examples/fsm.sv:5
i_wait PORT logic 1 INPUT examples/fsm.sv:7
o_state PORT [2:0] logic 3 OUTPUT examples/fsm.sv:8
fsm.i_clk VAR logic 1 INPUT examples/fsm.sv:3
fsm.i_finish VAR logic 1 INPUT examples/fsm.sv:6
fsm.i_rst VAR logic 1 INPUT examples/fsm.sv:4
Expand All @@ -108,12 +114,6 @@ and view a list of named objects in the FSM example module:
fsm.next_state VAR packed union 3 NONE examples/fsm.sv:31
fsm.o_state VAR [2:0] logic 3 OUTPUT examples/fsm.sv:8
fsm.state_q REG packed union 3 NONE examples/fsm.sv:28
i_clk VAR logic 1 INPUT examples/fsm.sv:3
i_finish VAR logic 1 INPUT examples/fsm.sv:6
i_rst VAR logic 1 INPUT examples/fsm.sv:4
i_start VAR logic 1 INPUT examples/fsm.sv:5
i_wait VAR logic 1 INPUT examples/fsm.sv:7
o_state VAR [2:0] logic 3 OUTPUT examples/fsm.sv:8
This output lists each of the variables in the design, their type (variable or
register), the Verilog data type, with data type width, the direction of the
Expand All @@ -126,10 +126,10 @@ expression matching):
➜ netlist-paths --compile examples/fsm.sv --dump-names state --regex
Name Type DType Width Direction Location
-------------- ---- ------------ ----- --------- ------------------
o_state PORT [2:0] logic 3 OUTPUT examples/fsm.sv:8
fsm.next_state VAR packed union 3 NONE examples/fsm.sv:31
fsm.o_state VAR [2:0] logic 3 OUTPUT examples/fsm.sv:8
fsm.state_q REG packed union 3 NONE examples/fsm.sv:28
o_state VAR [2:0] logic 3 OUTPUT examples/fsm.sv:8
There is similar behaviour with ``--dump-nets``, ``--dump-ports``,
``--dump-regs`` to select only net, port or register variable types
Expand Down Expand Up @@ -319,7 +319,8 @@ Developer notes
The command-line flags ``--verbose`` and ``--debug`` provide logging
information that can aid debugging.
To produce XML from a test case (noting the underlying call to Verilator):
To produce XML from a test case (noting the underlying call to Verilator),
using an adder as an example:
.. code-block:: bash
Expand Down
84 changes: 43 additions & 41 deletions include/netlist_paths/Vertex.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,40 +147,6 @@ inline const char *getVertexAstTypeStr(VertexAstType type) {
}
}

/// Map Vertex AST types to useful names that can be included in reports.
inline const char *getSimpleVertexAstTypeStr(VertexAstType type) {
switch (type) {
case VertexAstType::ALWAYS: return "ALWAYS";
case VertexAstType::ASSIGN: return "ASSIGN";
case VertexAstType::ASSIGN_ALIAS: return "ASSIGN";
case VertexAstType::ASSIGN_DLY: return "ASSIGN";
case VertexAstType::ASSIGN_W: return "ASSIGN";
case VertexAstType::CASE: return "CASE";
case VertexAstType::C_FUNC: return "C_FUNCTION";
case VertexAstType::C_METHOD_CALL: return "C_METHOD_CALL";
case VertexAstType::C_STMT: return "C_STATEMENT";
case VertexAstType::DISPLAY: return "DISPLAY";
case VertexAstType::DST_REG: return "REG";
case VertexAstType::DST_REG_ALIAS: return "REG_ALIAS";
case VertexAstType::FINISH: return "FINISH";
case VertexAstType::IF: return "IF";
case VertexAstType::INITIAL: return "INITIAL";
case VertexAstType::INSTANCE: return "INSTANCE";
case VertexAstType::INVALID: return "INVALID";
case VertexAstType::LOGIC: return "LOGIC";
case VertexAstType::PORT: return "PORT";
case VertexAstType::READ_MEM: return "READ_MEM";
case VertexAstType::SEN_GATE: return "SEN";
case VertexAstType::SFORMATF: return "SFORMATF";
case VertexAstType::SRC_REG: return "REG";
case VertexAstType::SRC_REG_ALIAS: return "REG_ALIAS";
case VertexAstType::VAR: return "VAR";
case VertexAstType::WHILE: return "WHILE";
case VertexAstType::WIRE: return "WIRE";
default: return "UNKNOWN";
}
}

inline VertexDirection getVertexDirection(const std::string &direction) {
static std::map<std::string, VertexDirection> mappings {
{ "input", VertexDirection::INPUT },
Expand Down Expand Up @@ -277,19 +243,19 @@ class Vertex {
top(v.top),
deleted(v.deleted) {}

/// Return whether a variable name is in the top scope.
/// Return whether a variable name is a top signal.
///
/// A variable is in the 'top' scope when has one or two hierarchical
/// components. For example, module.name or name is top level, but
/// module.submodule.name is not.
/// A variable is 'top' when it is not prefixed with any hierarchy path.
/// This applies to top-level ports and parameters only, since all other
/// variables exist within a module scope.
///
/// \param name The name of a variable.
///
/// \returns Whether the variable is in the top scope.
static bool determineIsTop(const std::string &name) {
std::vector<std::string> tokens;
boost::split(tokens, name, boost::is_any_of("."));
return tokens.size() < 3;
return tokens.size() == 1;
}

/// Given a hierarchical variable name, eg a.b.c, return the last component c.
Expand Down Expand Up @@ -416,8 +382,11 @@ class Vertex {
}

/// Return true if the vertex is a port variable.
/// Handle a special case where port registers have REG VertexAstType.
inline bool isPort() const {
return !deleted && astType == VertexAstType::PORT;
auto isAstPort = astType == VertexAstType::PORT;
auto isRegPort = top && isReg() && (direction == VertexDirection::OUTPUT);
return !deleted && (isAstPort || isRegPort);
}

/// Return true if the vertex is a valid start point for a combinatorial path
Expand Down Expand Up @@ -501,6 +470,40 @@ class Vertex {
!deleted;
}

/// Map Vertex AST types to useful names that can be included in reports.
const char *getSimpleAstTypeStr() const {
switch (astType) {
case VertexAstType::ALWAYS: return "ALWAYS";
case VertexAstType::ASSIGN: return "ASSIGN";
case VertexAstType::ASSIGN_ALIAS: return "ASSIGN";
case VertexAstType::ASSIGN_DLY: return "ASSIGN";
case VertexAstType::ASSIGN_W: return "ASSIGN";
case VertexAstType::CASE: return "CASE";
case VertexAstType::C_FUNC: return "C_FUNCTION";
case VertexAstType::C_METHOD_CALL: return "C_METHOD_CALL";
case VertexAstType::C_STMT: return "C_STATEMENT";
case VertexAstType::DISPLAY: return "DISPLAY";
case VertexAstType::DST_REG: return isPort() ? "PORT_REG" : "REG";
case VertexAstType::DST_REG_ALIAS: return "REG_ALIAS";
case VertexAstType::FINISH: return "FINISH";
case VertexAstType::IF: return "IF";
case VertexAstType::INITIAL: return "INITIAL";
case VertexAstType::INSTANCE: return "INSTANCE";
case VertexAstType::INVALID: return "INVALID";
case VertexAstType::LOGIC: return "LOGIC";
case VertexAstType::PORT: return "PORT";
case VertexAstType::READ_MEM: return "READ_MEM";
case VertexAstType::SEN_GATE: return "SEN";
case VertexAstType::SFORMATF: return "SFORMATF";
case VertexAstType::SRC_REG: return isPort() ? "PORT_REG" : "REG";
case VertexAstType::SRC_REG_ALIAS: return "REG_ALIAS";
case VertexAstType::VAR: return isPort() ? "PORT_VAR" : "VAR";
case VertexAstType::WHILE: return "WHILE";
case VertexAstType::WIRE: return "WIRE";
default: return "UNKNOWN";
}
}

/// Return a string description of this vertex.
std::string toString() const {
// TODO: expand on this for different vertex types.
Expand Down Expand Up @@ -530,7 +533,6 @@ class Vertex {
size_t getID() const { return id; }
const std::string getName() const { return name; }
const std::string getAstTypeStr() const { return getVertexAstTypeStr(astType); }
const std::string getSimpleAstTypeStr() const { return getSimpleVertexAstTypeStr(astType); }
const std::string getDirStr() const { return getVertexDirectionStr(direction); }
const std::string getDTypeStr() const { return dtype != nullptr ? dtype->toString() : "-"; }
const std::string getLocationStr() const { return location.getLocationStr(); }
Expand Down
4 changes: 2 additions & 2 deletions lib/netlist_paths/Graph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ Graph::getAllFanOut(VertexID startVertex) const {
Path(),
startVertex,
static_cast<VertexID>(v));
if (!path.empty()) {
if (path.length() > 1) {
path.reverse();
paths.push_back(path);
}
Expand Down Expand Up @@ -445,7 +445,7 @@ Graph::getAllFanIn(VertexID finishVertex) const {
Path(),
finishVertex,
static_cast<VertexID>(v));
if (!path.empty()) {
if (path.length() > 1) {
paths.push_back(path);
}
}
Expand Down
64 changes: 31 additions & 33 deletions tests/integration/py_wrapper_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,52 +30,50 @@ def test_verilator_bin(self):
def test_option_ignore_hierarchy_markers(self):
np = self.compile_test('counter.sv')
Options.get_instance().set_ignore_hierarchy_markers(True)
self.assertTrue(np.path_exists(Waypoints('counter/counter_q', 'counter/o_count')))
self.assertTrue(np.path_exists(Waypoints('counter_counter_q', 'counter_o_count')))
self.assertTrue(np.path_exists(Waypoints('counter/counter_q', 'o_count')))
self.assertTrue(np.path_exists(Waypoints('counter_counter_q', 'o_count')))

def test_adder_path_points(self):
"""
Test basic querying of path start and end points.
"""
np = self.compile_test('adder.sv')
for prefix in ['', 'adder.']:
# Start and finish points
self.assertTrue(np.startpoint_exists(prefix+'i_a'))
self.assertTrue(np.startpoint_exists(prefix+'i_b'))
self.assertTrue(np.endpoint_exists(prefix+'o_sum'))
self.assertTrue(np.endpoint_exists(prefix+'o_co'))
# Any start and finish points.
self.assertTrue(np.any_startpoint_exists(prefix+'i_a'))
self.assertTrue(np.any_startpoint_exists(prefix+'i_b'))
self.assertTrue(np.any_endpoint_exists(prefix+'o_sum'))
self.assertTrue(np.any_endpoint_exists(prefix+'o_co'))
Options.get_instance().set_match_regex()
self.assertTrue(np.any_startpoint_exists(prefix+'i_.*'))
self.assertTrue(np.any_endpoint_exists(prefix+'o_.*'))
Options.get_instance().set_match_exact()
# Invalid start and end points
self.assertFalse(np.endpoint_exists(prefix+'i_a'))
self.assertFalse(np.endpoint_exists(prefix+'i_b'))
self.assertFalse(np.startpoint_exists(prefix+'o_sum'))
self.assertFalse(np.startpoint_exists(prefix+'o_co'))
# Start and finish points
self.assertTrue(np.startpoint_exists('i_a'))
self.assertTrue(np.startpoint_exists('i_b'))
self.assertTrue(np.endpoint_exists('o_sum'))
self.assertTrue(np.endpoint_exists('o_co'))
# Any start and finish points.
self.assertTrue(np.any_startpoint_exists('i_a'))
self.assertTrue(np.any_startpoint_exists('i_b'))
self.assertTrue(np.any_endpoint_exists('o_sum'))
self.assertTrue(np.any_endpoint_exists('o_co'))
Options.get_instance().set_match_regex()
self.assertTrue(np.any_startpoint_exists('i_.*'))
self.assertTrue(np.any_endpoint_exists('o_.*'))
Options.get_instance().set_match_exact()
# Invalid start and end points
self.assertFalse(np.endpoint_exists('i_a'))
self.assertFalse(np.endpoint_exists('i_b'))
self.assertFalse(np.startpoint_exists('o_sum'))
self.assertFalse(np.startpoint_exists('o_co'))

def test_adder_path_exists(self):
"""
Test basic querying of path existence.
"""
np = self.compile_test('adder.sv')
Options.get_instance().set_match_exact()
for prefix in ['', 'adder.']:
# Check all valid paths are reported.
self.assertTrue(np.path_exists(Waypoints(prefix+'i_a', prefix+'o_sum')))
self.assertTrue(np.path_exists(Waypoints(prefix+'i_a', prefix+'o_co')))
self.assertTrue(np.path_exists(Waypoints(prefix+'i_b', prefix+'o_sum')))
self.assertTrue(np.path_exists(Waypoints(prefix+'i_b', prefix+'o_co')))
# Check for invalid paths.
self.assertRaises(RuntimeError, np.path_exists, Waypoints(prefix+'o_sum', prefix+'i_a'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints(prefix+'o_co', prefix+'i_a'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints(prefix+'o_sum', prefix+'i_b'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints(prefix+'o_co', prefix+'i_b'))
# Check all valid paths are reported.
self.assertTrue(np.path_exists(Waypoints('i_a', 'o_sum')))
self.assertTrue(np.path_exists(Waypoints('i_a', 'o_co')))
self.assertTrue(np.path_exists(Waypoints('i_b', 'o_sum')))
self.assertTrue(np.path_exists(Waypoints('i_b', 'o_co')))
# Check for invalid paths.
self.assertRaises(RuntimeError, np.path_exists, Waypoints('o_sum', 'i_a'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints('o_co', 'i_a'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints('o_sum', 'i_b'))
self.assertRaises(RuntimeError, np.path_exists, Waypoints('o_co', 'i_b'))

def test_get_names(self):
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/tool_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_option_ignore_hierarchy_markers(self):
test_path = os.path.join(defs.TEST_SRC_PREFIX, 'counter.sv')
returncode, stdout = self.run_np(['--compile', test_path,
'--from', 'counter/counter_q',
'--to', 'counter/o_count',
'--to', 'o_count',
'--ignore-hierarchy-markers'])
self.assertEqual(returncode, 0)

Expand Down
3 changes: 2 additions & 1 deletion tests/unit/NameTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ BOOST_AUTO_TEST_CASE(exact_name_matching_counter) {
BOOST_TEST(np->endpointExists("counter.counter_q"));
// Output ports can only be endpoints.
BOOST_TEST(!np->regExists("counter.o_count"));
BOOST_TEST(np->endpointExists("counter.o_count"));
BOOST_TEST(np->endpointExists("o_count"));
// Port enpoints are restricted to top instances, as above.
BOOST_TEST(!np->endpointExists("counter.o_count"));
// A name that doesn't exist.
BOOST_TEST(!np->regExists("foo"));
BOOST_TEST(!np->startpointExists("foo"));
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/PathAvoidTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ BOOST_AUTO_TEST_CASE(avoid_point_exception) {
}
{
auto waypoints = netlist_paths::Waypoints("in", "out");
waypoints.addThroughPoint("multiple_paths.in"); // Not a valid avoid point.
waypoints.addThroughPoint("in"); // Not a valid avoid point since it's a start point.
BOOST_CHECK_THROW(np->getAllPaths(waypoints), netlist_paths::Exception);
}
{
auto waypoints = netlist_paths::Waypoints("in", "out");
waypoints.addThroughPoint("multiple_paths.out"); // Not a valid avoid point.
waypoints.addThroughPoint("out"); // Not a valid avoid point since it's a finish point.
BOOST_CHECK_THROW(np->getAllPaths(waypoints), netlist_paths::Exception);
}
}
Expand Down
22 changes: 11 additions & 11 deletions tests/unit/PathExistsTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ BOOST_FIXTURE_TEST_SUITE(path_exists_query, TestContext);
BOOST_AUTO_TEST_CASE(path_exists_adder) {
BOOST_CHECK_NO_THROW(compile("adder.sv"));
// Check paths between all start and end points are reported.
auto startPoints = {"adder.i_a", "adder.i_b"};
auto endPoints = {"adder.o_sum", "adder.o_co"};
auto startPoints = {"i_a", "i_b"};
auto endPoints = {"o_sum", "o_co"};
for (auto s : startPoints) {
for (auto e : endPoints) {
BOOST_TEST(np->startpointExists(s));
Expand All @@ -28,19 +28,19 @@ BOOST_AUTO_TEST_CASE(path_exists_adder) {
BOOST_AUTO_TEST_CASE(path_exists_counter) {
BOOST_CHECK_NO_THROW(compile("counter.sv"));
// Valid paths.
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.i_clk", "counter.counter_q")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.i_rst", "counter.counter_q")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.counter_q", "counter.o_count")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.counter_q", "counter.o_wrap")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("i_clk", "counter.counter_q")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("i_rst", "counter.counter_q")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.counter_q", "o_count")));
BOOST_TEST(np->pathExists(netlist_paths::Waypoints("counter.counter_q", "o_wrap")));
// Invalid paths (invalid start/end points).
BOOST_CHECK_THROW(np->pathExists(netlist_paths::Waypoints("counter.o_count", "counter.counter_q")), netlist_paths::Exception);
BOOST_CHECK_THROW(np->pathExists(netlist_paths::Waypoints("o_count", "counter.counter_q")), netlist_paths::Exception);
BOOST_CHECK_THROW(np->pathExists(netlist_paths::Waypoints("counter.count_q", "counter.i_clk")), netlist_paths::Exception);
BOOST_CHECK_THROW(np->pathExists(netlist_paths::Waypoints("counter.count_q", "counter.i_rst")), netlist_paths::Exception);
// Invalid paths (valid start/end points).
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("counter.i_clk", "counter.o_count")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("counter.i_clk", "counter.o_wrap")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("counter.i_rst", "counter.o_count")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("counter.i_rst", "counter.o_wrap")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("i_clk", "o_count")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("i_clk", "o_wrap")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("i_rst", "o_count")));
BOOST_TEST(!np->pathExists(netlist_paths::Waypoints("i_rst", "o_wrap")));
// TODO: check --from o_counter has no fan out paths
}

Expand Down
Loading

0 comments on commit 9dedf78

Please sign in to comment.