From 3a7a223856180ea2a0045e1b77089bfd9a17a47f Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 4 Aug 2023 09:07:34 +0100 Subject: [PATCH 01/31] WIP --- tools/netlist/TODO.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index f72c05800..40f53b68c 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -36,6 +36,12 @@ To dos RandSequenceStatement ProceduralCheckerStatement +- In SplitVariables, handle: + * Nested range selections, eg `x[3:0][2:0][1:0]` has a dependency with + `x[0]`, so effectively multiple ranges should be flattened eg to `[1:0]`. + * Unions, where they casue access cannot to no longer be tracked through a + heirarchy of types. Instead access must be resolved on the bit level (bit blasted). + - Dumping of a dot file outputs random characters at the end. - Reporting of variables in the netlist (by type, matching patterns). - Infer sequential elements in the netlist (ie non-blocking assignment and From 52ee60d94d79966316fe1aa1347ca678eb4d2124 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Wed, 9 Aug 2023 22:07:05 +0100 Subject: [PATCH 02/31] WIP --- tools/netlist/include/Netlist.h | 4 +- tools/netlist/include/SplitVariables.h | 191 ++++++++++++++++++++----- 2 files changed, 155 insertions(+), 40 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 915dcc2d8..10d7b95c3 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -110,9 +110,9 @@ struct VariableRangeSelect : public VariableSelectorBase { /// A variable selector representing member access of a structure. struct VariableMemberAccess : public VariableSelectorBase { - std::string_view name; + const std::string_view name; - VariableMemberAccess(std::string_view name) : + VariableMemberAccess(const std::string_view name) : VariableSelectorBase(VariableSelectorKind::MemberAccess), name(name) {} static bool isKind(VariableSelectorKind otherKind) { diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 611ebb8bf..3a0405a9b 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -13,9 +13,11 @@ #include "fmt/format.h" #include +#include "slang/ast/types/AllTypes.h" #include "slang/ast/types/Type.h" #include "slang/util/Util.h" + namespace netlist { /// A class to perform a transformation on the netlist to split variable @@ -26,6 +28,119 @@ class SplitVariables { SplitVariables(Netlist& netlist) : netlist(netlist) { split(); } private: + + /// Given a packed struct type, return the bit position of the named field. + std::pair getFieldRange(const slang::ast::PackedStructType &packedStruct, + const std::string_view fieldName) const { + size_t offset = 0; + for (auto &member : packedStruct.members()) { + auto fieldWidth = member.getDeclaredType()->getType().getBitWidth(); + if (member.name == fieldName) { + return std::make_pair(offset, offset+fieldWidth); + }; + offset += fieldWidth;; + } + SLANG_UNREACHABLE; + } + + /// Given a variable reference with zero or more selectors, determine the + /// bit range that is accessed. + /// Call with: auto& type = node.symbol.getDeclaredType()->getType(); + std::pair getBitRange(const NetlistVariableReference& node, + const slang::ast::Type &type, + NetlistVariableReference::SelectorsListType::iterator selectorsIt, + size_t leftIndex, size_t rightIndex) { + // No selectors + if (node.selectors.empty()) { + return std::make_pair(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); + } + // Scalar + if (type.isScalar()) { + if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { + const auto& elementSelector = selectorsIt->get()->as(); + SLANG_ASSERT(elementSelector.getIndexInt() < rightIndex - leftIndex); + size_t index = leftIndex + elementSelector.getIndexInt(); + return std::make_pair(index, index); + } + else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { + const auto& rangeSelector = selectorsIt->get()->as(); + SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); + SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); + SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); + if (std::next(selectorsIt) != node.selectors.end()) { + return getBitRange(node, type, std::next(selectorsIt), + leftIndex + rangeSelector.getLeftIndexInt(), + leftIndex + rangeSelector.getRightIndexInt()); + } else { + return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), + leftIndex + rangeSelector.getRightIndexInt()); + } + } + else { + SLANG_ASSERT(0 && "unsupported scalar selector"); + } + } + // Packed or unpacked array + else if (type.isArray()) { + if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { + const auto& elementSelector = selectorsIt->get()->as(); + size_t index = leftIndex + elementSelector.getIndexInt(); + return std::make_pair(index, index); + } + else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { + const auto& rangeSelector = selectorsIt->get()->as(); + SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); + SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); + SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); + if (std::next(selectorsIt) != node.selectors.end()) { + return getBitRange(node, type, std::next(selectorsIt), + leftIndex + rangeSelector.getLeftIndexInt(), + leftIndex + rangeSelector.getRightIndexInt()); + } else { + return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), + leftIndex + rangeSelector.getRightIndexInt()); + } + } + else { + SLANG_ASSERT(0 && "unsupported array selector"); + } + } + // Packed struct + else if (type.isStruct() && !type.isUnpackedStruct()) { + const auto& packedStruct = type.getCanonicalType().as(); + SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); + const auto& memberAccessSelector = selectorsIt->get()->as(); + auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); + SLANG_ASSERT(fieldRange.first >= leftIndex); + SLANG_ASSERT(fieldRange.second <= rightIndex); + auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; + return getBitRange(node, fieldType, std::next(selectorsIt), + leftIndex + fieldRange.first, + leftIndex + fieldRange.second); + } + else if (type.isPackedUnion()) { + } + else if (type.isEnum()) { + } + else { + SLANG_THROW(std::runtime_error("unhandled type")); + } + + //for (auto& selector : node.selectors) { + // switch (selector->kind) { + // case VariableSelectorKind::ElementSelect: { + // break; + // } + // case VariableSelectorKind::RangeSelect: { + // break; + // } + // case VariableSelectorKind::MemberAccess: { + // break; + // } + // } + //} + } + /// Given two ranges [end1:start1] and [end2:start2], return true if there is /// any overlap in values between them. inline bool rangesOverlap(size_t start1, size_t end1, size_t start2, size_t end2) const { @@ -36,44 +151,44 @@ class SplitVariables { /// selection made by the source node. bool isIntersectingSelection(NetlistVariableReference& sourceNode, NetlistVariableReference& targetNode) const { - bool match = true; - size_t selectorDepth = 0; - while (match) { - // Terminate the loop if either variable reference has no further - // selectors. - if (selectorDepth >= sourceNode.selectors.size() || - selectorDepth >= targetNode.selectors.size()) { - break; - } - auto& sourceSelector = sourceNode.selectors[selectorDepth]; - auto& targetSelector = targetNode.selectors[selectorDepth]; - SLANG_ASSERT(sourceSelector->kind == targetSelector->kind && "selectors do not match"); - switch (sourceSelector->kind) { - case VariableSelectorKind::ElementSelect: - // Matching selectors if the index is the same. - match = sourceSelector->as().getIndexInt() == - targetSelector->as().getIndexInt(); - break; - case VariableSelectorKind::RangeSelect: { - // Matching selectors if there is any overlap in the two ranges. - auto sourceRangeSel = sourceSelector->as(); - auto targetRangeSel = targetSelector->as(); - auto srcLeft = sourceRangeSel.getLeftIndexInt(); - auto srcRight = sourceRangeSel.getRightIndexInt(); - auto tgtLeft = targetRangeSel.getLeftIndexInt(); - auto tgtRight = targetRangeSel.getRightIndexInt(); - match = rangesOverlap(srcRight, srcLeft, tgtRight, tgtLeft); - break; - } - case VariableSelectorKind::MemberAccess: - // Matching selectors if the member names match. - match = sourceSelector->as().name == - targetSelector->as().name; - break; - } - selectorDepth++; - } - return match; + //bool match = true; + //size_t selectorDepth = 0; + //while (match) { + // // Terminate the loop if either variable reference has no further + // // selectors. + // if (selectorDepth >= sourceNode.selectors.size() || + // selectorDepth >= targetNode.selectors.size()) { + // break; + // } + // auto& sourceSelector = sourceNode.selectors[selectorDepth]; + // auto& targetSelector = targetNode.selectors[selectorDepth]; + // SLANG_ASSERT(sourceSelector->kind == targetSelector->kind && "selectors do not match"); + // switch (sourceSelector->kind) { + // case VariableSelectorKind::ElementSelect: + // // Matching selectors if the index is the same. + // match = sourceSelector->as().getIndexInt() == + // targetSelector->as().getIndexInt(); + // break; + // case VariableSelectorKind::RangeSelect: { + // // Matching selectors if there is any overlap in the two ranges. + // auto sourceRangeSel = sourceSelector->as(); + // auto targetRangeSel = targetSelector->as(); + // auto srcLeft = sourceRangeSel.getLeftIndexInt(); + // auto srcRight = sourceRangeSel.getRightIndexInt(); + // auto tgtLeft = targetRangeSel.getLeftIndexInt(); + // auto tgtRight = targetRangeSel.getRightIndexInt(); + // match = rangesOverlap(srcRight, srcLeft, tgtRight, tgtLeft); + // break; + // } + // case VariableSelectorKind::MemberAccess: + // // Matching selectors if the member names match. + // match = sourceSelector->as().name == + // targetSelector->as().name; + // break; + // } + // selectorDepth++; + //} + //return match; } void split() { From a346ee813dd5c2319fe81eab82bfb816cca6974d Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 13 Aug 2023 21:20:25 +0100 Subject: [PATCH 03/31] WIP --- tools/netlist/CMakeLists.txt | 3 +- tools/netlist/include/Netlist.h | 6 +- tools/netlist/include/NetlistVisitor.h | 2 + tools/netlist/include/SplitVariables.h | 256 +++++++------- tools/netlist/source/Netlist.cpp | 6 + tools/netlist/tests/CMakeLists.txt | 4 +- tools/netlist/tests/NetlistTest.h | 28 ++ tools/netlist/tests/NetlistTests.cpp | 259 +------------- .../netlist/tests/VariableSelectorsTests.cpp | 328 ++++++++++++++++++ 9 files changed, 493 insertions(+), 399 deletions(-) create mode 100644 tools/netlist/source/Netlist.cpp create mode 100644 tools/netlist/tests/NetlistTest.h create mode 100644 tools/netlist/tests/VariableSelectorsTests.cpp diff --git a/tools/netlist/CMakeLists.txt b/tools/netlist/CMakeLists.txt index c30514592..80efb72c2 100644 --- a/tools/netlist/CMakeLists.txt +++ b/tools/netlist/CMakeLists.txt @@ -2,7 +2,8 @@ # SPDX-FileCopyrightText: Michael Popoloski # SPDX-License-Identifier: MIT # ~~~ -add_executable(slang_netlist netlist.cpp) + +add_executable(slang_netlist netlist.cpp source/Netlist.cpp) add_executable(slang::netlist ALIAS slang_netlist) target_link_libraries( diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 10d7b95c3..74c3797f7 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -46,6 +46,10 @@ struct VariableSelectorBase { virtual ~VariableSelectorBase() = default; virtual std::string toString() const = 0; + bool isElementSelect() const { return kind == VariableSelectorKind::ElementSelect; } + bool isRangeSelect() const { return kind == VariableSelectorKind::RangeSelect; } + bool isMemberAccess() const { return kind == VariableSelectorKind::MemberAccess; } + template T& as() { SLANG_ASSERT(T::isKind(kind)); @@ -176,8 +180,6 @@ class NetlistNode : public Node { static size_t nextID; }; -size_t NetlistNode::nextID = 0; - /// A class representing a port declaration. class NetlistPortDeclaration : public NetlistNode { public: diff --git a/tools/netlist/include/NetlistVisitor.h b/tools/netlist/include/NetlistVisitor.h index 3d6a3298a..878b1ad95 100644 --- a/tools/netlist/include/NetlistVisitor.h +++ b/tools/netlist/include/NetlistVisitor.h @@ -13,6 +13,7 @@ #include "Netlist.h" #include "fmt/color.h" #include "fmt/format.h" +#include #include #include "slang/ast/ASTVisitor.h" @@ -85,6 +86,7 @@ class VariableReferenceVisitor : public ast::ASTVisitoras().member.name); } } + std::reverse(node.selectors.begin(), node.selectors.end()); selectors.clear(); } diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 3a0405a9b..a2b56e8cb 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -20,175 +20,157 @@ namespace netlist { -/// A class to perform a transformation on the netlist to split variable -/// declaration nodes of structured types into multiple parts based on the -/// types of the incoming and outgoing edges. -class SplitVariables { -public: - SplitVariables(Netlist& netlist) : netlist(netlist) { split(); } +struct BitRange { + size_t start; + size_t end; + BitRange(size_t index) : start(index), end(index) {} + BitRange(size_t start, size_t end) : start(start), end(end) { + SLANG_ASSERT(start <= end); + } + /// Given two ranges, return true if there is any overlap in values between them. + inline bool overlap(BitRange other) const { + return start <= other.end && other.start <= end; + } + size_t size() const { return end - start; } +}; +class AnalyseVariableReference { private: + NetlistVariableReference& node; + NetlistVariableReference::SelectorsListType::iterator selectorsIt; + +public: + AnalyseVariableReference(NetlistVariableReference &node) : + node(node), selectorsIt(node.selectors.begin()) { + } /// Given a packed struct type, return the bit position of the named field. - std::pair getFieldRange(const slang::ast::PackedStructType &packedStruct, - const std::string_view fieldName) const { + BitRange getFieldRange(const slang::ast::PackedStructType &packedStruct, + const std::string_view fieldName) const { size_t offset = 0; for (auto &member : packedStruct.members()) { - auto fieldWidth = member.getDeclaredType()->getType().getBitWidth(); - if (member.name == fieldName) { - return std::make_pair(offset, offset+fieldWidth); - }; - offset += fieldWidth;; + auto fieldWidth = member.getDeclaredType()->getType().getBitWidth(); + if (member.name == fieldName) { + return BitRange(offset, offset+fieldWidth); + }; + offset += fieldWidth;; } SLANG_UNREACHABLE; } + BitRange handleScalarElementSelect(const slang::ast::Type &type, BitRange range) { + const auto& elementSelector = selectorsIt->get()->as(); + SLANG_ASSERT(elementSelector.getIndexInt() >= range.start); + SLANG_ASSERT(elementSelector.getIndexInt() <= range.end); + size_t index = range.start + elementSelector.getIndexInt(); + return BitRange(index); + } + + BitRange handleScalarRangeSelect(const slang::ast::Type &type, BitRange range) { + const auto& rangeSelector = selectorsIt->get()->as(); + SLANG_ASSERT(rangeSelector.getRightIndexInt() >= range.start); + SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= range.end); + SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rangeSelector.getLeftIndexInt()); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + return getBitRangeImpl(type, + BitRange(rangeSelector.getRightIndexInt(), + rangeSelector.getLeftIndexInt())); + } else { + return BitRange(rangeSelector.getRightIndexInt(), + rangeSelector.getLeftIndexInt()); + } + } + /// Given a variable reference with zero or more selectors, determine the /// bit range that is accessed. - /// Call with: auto& type = node.symbol.getDeclaredType()->getType(); - std::pair getBitRange(const NetlistVariableReference& node, - const slang::ast::Type &type, - NetlistVariableReference::SelectorsListType::iterator selectorsIt, - size_t leftIndex, size_t rightIndex) { + BitRange getBitRangeImpl(const slang::ast::Type &type, BitRange range) { // No selectors if (node.selectors.empty()) { - return std::make_pair(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); + return BitRange(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); } // Scalar - if (type.isScalar()) { - if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { - const auto& elementSelector = selectorsIt->get()->as(); - SLANG_ASSERT(elementSelector.getIndexInt() < rightIndex - leftIndex); - size_t index = leftIndex + elementSelector.getIndexInt(); - return std::make_pair(index, index); - } - else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { - const auto& rangeSelector = selectorsIt->get()->as(); - SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); - SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); - SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); - if (std::next(selectorsIt) != node.selectors.end()) { - return getBitRange(node, type, std::next(selectorsIt), - leftIndex + rangeSelector.getLeftIndexInt(), - leftIndex + rangeSelector.getRightIndexInt()); - } else { - return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), - leftIndex + rangeSelector.getRightIndexInt()); - } - } - else { - SLANG_ASSERT(0 && "unsupported scalar selector"); + if (type.isSimpleBitVector()) { + if (selectorsIt->get()->isElementSelect()) { + return handleScalarElementSelect(type, range); } - } - // Packed or unpacked array - else if (type.isArray()) { - if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { - const auto& elementSelector = selectorsIt->get()->as(); - size_t index = leftIndex + elementSelector.getIndexInt(); - return std::make_pair(index, index); - } - else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { - const auto& rangeSelector = selectorsIt->get()->as(); - SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); - SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); - SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); - if (std::next(selectorsIt) != node.selectors.end()) { - return getBitRange(node, type, std::next(selectorsIt), - leftIndex + rangeSelector.getLeftIndexInt(), - leftIndex + rangeSelector.getRightIndexInt()); - } else { - return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), - leftIndex + rangeSelector.getRightIndexInt()); - } + else if (selectorsIt->get()->isRangeSelect()) { + return handleScalarRangeSelect(type, range); } else { - SLANG_ASSERT(0 && "unsupported array selector"); + SLANG_ASSERT(0 && "unsupported scalar selector"); } } - // Packed struct - else if (type.isStruct() && !type.isUnpackedStruct()) { - const auto& packedStruct = type.getCanonicalType().as(); - SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); - const auto& memberAccessSelector = selectorsIt->get()->as(); - auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); - SLANG_ASSERT(fieldRange.first >= leftIndex); - SLANG_ASSERT(fieldRange.second <= rightIndex); - auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; - return getBitRange(node, fieldType, std::next(selectorsIt), - leftIndex + fieldRange.first, - leftIndex + fieldRange.second); - } - else if (type.isPackedUnion()) { - } - else if (type.isEnum()) { - } + //// Packed or unpacked array + //else if (type.isArray()) { + // if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { + // const auto& elementSelector = selectorsIt->get()->as(); + // size_t index = leftIndex + elementSelector.getIndexInt(); + // // TODO: also needs to recurse if there are more selectors + // return std::make_pair(index, index); + // } + // else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { + // const auto& rangeSelector = selectorsIt->get()->as(); + // SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); + // SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); + // SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); + // if (std::next(selectorsIt) != node.selectors.end()) { + // return getBitRange(node, type, std::next(selectorsIt), + // leftIndex + rangeSelector.getLeftIndexInt(), + // leftIndex + rangeSelector.getRightIndexInt()); + // } else { + // return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), + // leftIndex + rangeSelector.getRightIndexInt()); + // } + // } + // else { + // SLANG_ASSERT(0 && "unsupported array selector"); + // } + //} + //// Packed struct + //else if (type.isStruct() && !type.isUnpackedStruct()) { + // const auto& packedStruct = type.getCanonicalType().as(); + // SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); + // const auto& memberAccessSelector = selectorsIt->get()->as(); + // auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); + // SLANG_ASSERT(fieldRange.first >= leftIndex); + // SLANG_ASSERT(fieldRange.second <= rightIndex); + // auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; + // return getBitRange(node, fieldType, std::next(selectorsIt), + // leftIndex + fieldRange.first, + // leftIndex + fieldRange.second); + //} + //else if (type.isPackedUnion()) { + //} + //else if (type.isEnum()) { + //} else { SLANG_THROW(std::runtime_error("unhandled type")); } - - //for (auto& selector : node.selectors) { - // switch (selector->kind) { - // case VariableSelectorKind::ElementSelect: { - // break; - // } - // case VariableSelectorKind::RangeSelect: { - // break; - // } - // case VariableSelectorKind::MemberAccess: { - // break; - // } - // } - //} } - /// Given two ranges [end1:start1] and [end2:start2], return true if there is - /// any overlap in values between them. - inline bool rangesOverlap(size_t start1, size_t end1, size_t start2, size_t end2) const { - return start1 <= end2 && start2 <= end1; + /// Return a range indicating the bits of the variable that are accessed. + BitRange getBitRange() { + auto& variableType = node.symbol.getDeclaredType()->getType(); + return getBitRangeImpl(variableType, BitRange(0, variableType.getBitWidth()-1)); } +}; + +/// A class to perform a transformation on the netlist to split variable +/// declaration nodes of structured types into multiple parts based on the +/// types of the incoming and outgoing edges. +class SplitVariables { +public: + SplitVariables(Netlist& netlist) : netlist(netlist) { split(); } + +private: /// Return true if the selection made by the target node intersects with the /// selection made by the source node. bool isIntersectingSelection(NetlistVariableReference& sourceNode, NetlistVariableReference& targetNode) const { - //bool match = true; - //size_t selectorDepth = 0; - //while (match) { - // // Terminate the loop if either variable reference has no further - // // selectors. - // if (selectorDepth >= sourceNode.selectors.size() || - // selectorDepth >= targetNode.selectors.size()) { - // break; - // } - // auto& sourceSelector = sourceNode.selectors[selectorDepth]; - // auto& targetSelector = targetNode.selectors[selectorDepth]; - // SLANG_ASSERT(sourceSelector->kind == targetSelector->kind && "selectors do not match"); - // switch (sourceSelector->kind) { - // case VariableSelectorKind::ElementSelect: - // // Matching selectors if the index is the same. - // match = sourceSelector->as().getIndexInt() == - // targetSelector->as().getIndexInt(); - // break; - // case VariableSelectorKind::RangeSelect: { - // // Matching selectors if there is any overlap in the two ranges. - // auto sourceRangeSel = sourceSelector->as(); - // auto targetRangeSel = targetSelector->as(); - // auto srcLeft = sourceRangeSel.getLeftIndexInt(); - // auto srcRight = sourceRangeSel.getRightIndexInt(); - // auto tgtLeft = targetRangeSel.getLeftIndexInt(); - // auto tgtRight = targetRangeSel.getRightIndexInt(); - // match = rangesOverlap(srcRight, srcLeft, tgtRight, tgtLeft); - // break; - // } - // case VariableSelectorKind::MemberAccess: - // // Matching selectors if the member names match. - // match = sourceSelector->as().name == - // targetSelector->as().name; - // break; - // } - // selectorDepth++; - //} - //return match; + return AnalyseVariableReference(sourceNode).getBitRange().overlap( + AnalyseVariableReference(targetNode).getBitRange()); } void split() { diff --git a/tools/netlist/source/Netlist.cpp b/tools/netlist/source/Netlist.cpp new file mode 100644 index 000000000..60fc25ea6 --- /dev/null +++ b/tools/netlist/source/Netlist.cpp @@ -0,0 +1,6 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "Netlist.h" + +size_t netlist::NetlistNode::nextID = 0; diff --git a/tools/netlist/tests/CMakeLists.txt b/tools/netlist/tests/CMakeLists.txt index cc645cfdf..f0b38c2ac 100644 --- a/tools/netlist/tests/CMakeLists.txt +++ b/tools/netlist/tests/CMakeLists.txt @@ -6,7 +6,9 @@ add_executable( netlist_unittests ../../../tests/unittests/main.cpp ../../../tests/unittests/Test.cpp - DepthFirstSearchTests.cpp DirectedGraphTests.cpp NetlistTests.cpp) + ../source/Netlist.cpp + DepthFirstSearchTests.cpp DirectedGraphTests.cpp NetlistTests.cpp + VariableSelectorsTests.cpp) target_link_libraries( netlist_unittests diff --git a/tools/netlist/tests/NetlistTest.h b/tools/netlist/tests/NetlistTest.h new file mode 100644 index 000000000..33d94fc07 --- /dev/null +++ b/tools/netlist/tests/NetlistTest.h @@ -0,0 +1,28 @@ +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT + +#include "Netlist.h" +#include "NetlistVisitor.h" +#include "PathFinder.h" +#include "SplitVariables.h" +#include "Test.h" + +#include + +using namespace netlist; + +inline Netlist createNetlist(Compilation& compilation) { + Netlist netlist; + NetlistVisitor visitor(compilation, netlist); + compilation.getRoot().visit(visitor); + SplitVariables splitVariables(netlist); + return netlist; +} + +inline bool pathExists(Netlist &netlist, const std::string& startName, const std::string& endName) { + PathFinder pathFinder(netlist); + auto* startNode = netlist.lookupPort(startName); + auto* endNode = netlist.lookupPort(endName); + return !pathFinder.find(*startNode, *endNode).empty(); +} + diff --git a/tools/netlist/tests/NetlistTests.cpp b/tools/netlist/tests/NetlistTests.cpp index a9ac24ab3..19cc68522 100644 --- a/tools/netlist/tests/NetlistTests.cpp +++ b/tools/netlist/tests/NetlistTests.cpp @@ -6,21 +6,7 @@ // SPDX-License-Identifier: MIT //------------------------------------------------------------------------------ -#include "Netlist.h" -#include "NetlistVisitor.h" -#include "PathFinder.h" -#include "SplitVariables.h" -#include "Test.h" - -using namespace netlist; - -Netlist createNetlist(Compilation& compilation) { - Netlist netlist; - NetlistVisitor visitor(compilation, netlist); - compilation.getRoot().visit(visitor); - SplitVariables splitVariables(netlist); - return netlist; -} +#include "NetlistTest.h" //===---------------------------------------------------------------------===// // Basic tests @@ -168,249 +154,6 @@ endmodule CHECK(*path.findVariable("nested_passthrough.foo_b.o_value") == 6); } -//===---------------------------------------------------------------------===// -// Tests for variable splitting -//===---------------------------------------------------------------------===// - -TEST_CASE("Chain of assignments in a sequence using a vector") { - // As above but this time using a packed array. - auto tree = SyntaxTree::fromText(R"( -module chain_array (input logic i_value, output logic o_value); - - logic [4:0] x; - - assign x[0] = i_value; - - always_comb begin - x[1] = x[0]; - x[2] = x[1]; - x[3] = x[2]; - end - - assign x[4] = x[3]; - assign o_value = x[4]; - -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - auto path = pathFinder.find(*netlist.lookupPort("chain_array.i_value"), - *netlist.lookupPort("chain_array.o_value")); - CHECK(*path.findVariable("chain_array.x[0]") == 1); - CHECK(*path.findVariable("chain_array.x[1]") == 3); - CHECK(*path.findVariable("chain_array.x[2]") == 5); - CHECK(*path.findVariable("chain_array.x[3]") == 7); - CHECK(*path.findVariable("chain_array.x[4]") == 9); -} - -TEST_CASE("Passthrough two signals via a shared structure") { - auto tree = SyntaxTree::fromText(R"( -module passthrough_member_access ( - input logic i_value_a, - input logic i_value_b, - output logic o_value_a, - output logic o_value_b -); - - struct packed { - logic a; - logic b; - } foo; - - assign foo.a = i_value_a; - assign foo.b = i_value_b; - - assign o_value_a = foo.a; - assign o_value_b = foo.b; - -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - auto* inPortA = netlist.lookupPort("passthrough_member_access.i_value_a"); - auto* inPortB = netlist.lookupPort("passthrough_member_access.i_value_b"); - auto* outPortA = netlist.lookupPort("passthrough_member_access.o_value_a"); - auto* outPortB = netlist.lookupPort("passthrough_member_access.o_value_b"); - PathFinder pathFinder(netlist); - // Valid paths. - CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); - CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); - // Invalid paths. - CHECK(pathFinder.find(*inPortA, *outPortB).empty()); - CHECK(pathFinder.find(*inPortB, *outPortA).empty()); -} - -TEST_CASE("Passthrough two signals via ranges in a shared vector") { - auto tree = SyntaxTree::fromText(R"( -module passthrough_ranges ( - input logic [1:0] i_value_a, - input logic [1:0] i_value_b, - output logic [1:0] o_value_a, - output logic [1:0] o_value_b -); - - logic [3:0] foo; - - assign foo[1:0] = i_value_a; - assign foo[3:2] = i_value_b; - - assign o_value_a = foo[1:0]; - assign o_value_b = foo[3:2]; - -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - auto* inPortA = netlist.lookupPort("passthrough_ranges.i_value_a"); - auto* inPortB = netlist.lookupPort("passthrough_ranges.i_value_b"); - auto* outPortA = netlist.lookupPort("passthrough_ranges.o_value_a"); - auto* outPortB = netlist.lookupPort("passthrough_ranges.o_value_b"); - PathFinder pathFinder(netlist); - // Valid paths. - CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); - CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); - // Invalid paths. - CHECK(pathFinder.find(*inPortA, *outPortB).empty()); - CHECK(pathFinder.find(*inPortB, *outPortA).empty()); -} - -//===---------------------------------------------------------------------===// -// Tests for loop unrolling -//===---------------------------------------------------------------------===// - -TEST_CASE("Chain of assignments using a single loop") { - // Test that a loop can be unrolled and variable declarations can be split - // out for elements of an array. - auto tree = SyntaxTree::fromText(R"( -module chain_array #(parameter N=4) ( - input logic i_value, - output logic o_value); - - logic pipeline [N-1:0]; - - assign pipeline[0] = i_value; - - always_comb begin - for (int i=1; i o_value, check it passes through each stage. - CHECK(pathFinder - .find(*netlist.lookupPort("chain_array.i_value"), - *netlist.lookupPort("chain_array.o_value")) - .size() == 10); -} - -TEST_CASE("Chain of assignments using a nested loop") { - // Expand the previous test to check the handling of multiple loop variables. - auto tree = SyntaxTree::fromText(R"( -module chain_nested #(parameter N=3) ( - input logic i_value, - output logic o_value); - - logic [(N*N)-1:0] stages; - - //assign stages[0] = i_value; - assign o_value = stages[(N*N)-1]; - - always_comb begin - for (int i=0; i o_value, check it passes through each stage. - auto path = pathFinder.find(*netlist.lookupPort("chain_nested.i_value"), - *netlist.lookupPort("chain_nested.o_value")); - CHECK(*path.findVariable("chain_nested.stages[0]") == 1); - CHECK(*path.findVariable("chain_nested.stages[1]") == 3); - CHECK(*path.findVariable("chain_nested.stages[2]") == 5); - CHECK(*path.findVariable("chain_nested.stages[3]") == 7); - CHECK(*path.findVariable("chain_nested.stages[4]") == 9); - CHECK(*path.findVariable("chain_nested.stages[5]") == 11); - CHECK(*path.findVariable("chain_nested.stages[6]") == 13); - CHECK(*path.findVariable("chain_nested.stages[7]") == 15); - CHECK(*path.findVariable("chain_nested.stages[8]") == 17); -} - -TEST_CASE("Two chains of assignments using a shared 2D array") { - // Check that assignments corresponding to two distinct paths, using the same - // array variable can be handled. - auto tree = SyntaxTree::fromText(R"( -module chain_loop_dual #(parameter N=4)( - input logic i_value_a, - input logic i_value_b, - output logic o_value_a, - output logic o_value_b -); - - parameter A = 0; - parameter B = 1; - - logic pipeline [1:0][N-1:0]; - - assign pipeline[A][0] = i_value_a; - assign pipeline[B][0] = i_value_b; - - always_comb begin - for (int i=1; i o_value, check it passes through each stage. +// CHECK(pathFinder +// .find(*netlist.lookupPort("chain_array.i_value"), +// *netlist.lookupPort("chain_array.o_value")) +// .size() == 10); +//} +// +//TEST_CASE("Chain of assignments using a nested loop") { +// // Expand the previous test to check the handling of multiple loop variables. +// auto tree = SyntaxTree::fromText(R"( +//module chain_nested #(parameter N=3) ( +// input logic i_value, +// output logic o_value); +// +// logic [(N*N)-1:0] stages; +// +// //assign stages[0] = i_value; +// assign o_value = stages[(N*N)-1]; +// +// always_comb begin +// for (int i=0; i o_value, check it passes through each stage. +// auto path = pathFinder.find(*netlist.lookupPort("chain_nested.i_value"), +// *netlist.lookupPort("chain_nested.o_value")); +// CHECK(*path.findVariable("chain_nested.stages[0]") == 1); +// CHECK(*path.findVariable("chain_nested.stages[1]") == 3); +// CHECK(*path.findVariable("chain_nested.stages[2]") == 5); +// CHECK(*path.findVariable("chain_nested.stages[3]") == 7); +// CHECK(*path.findVariable("chain_nested.stages[4]") == 9); +// CHECK(*path.findVariable("chain_nested.stages[5]") == 11); +// CHECK(*path.findVariable("chain_nested.stages[6]") == 13); +// CHECK(*path.findVariable("chain_nested.stages[7]") == 15); +// CHECK(*path.findVariable("chain_nested.stages[8]") == 17); +//} +// +//TEST_CASE("Two chains of assignments using a shared 2D array") { +// // Check that assignments corresponding to two distinct paths, using the same +// // array variable can be handled. +// auto tree = SyntaxTree::fromText(R"( +//module chain_loop_dual #(parameter N=4)( +// input logic i_value_a, +// input logic i_value_b, +// output logic o_value_a, +// output logic o_value_b +//); +// +// parameter A = 0; +// parameter B = 1; +// +// logic pipeline [1:0][N-1:0]; +// +// assign pipeline[A][0] = i_value_a; +// assign pipeline[B][0] = i_value_b; +// +// always_comb begin +// for (int i=1; i Date: Mon, 14 Aug 2023 21:04:59 +0100 Subject: [PATCH 04/31] WIP --- tools/netlist/include/SplitVariables.h | 170 ++++++++++++------ .../netlist/tests/VariableSelectorsTests.cpp | 102 ++++++++++- 2 files changed, 215 insertions(+), 57 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index a2b56e8cb..1c0c2c833 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -21,16 +21,20 @@ namespace netlist { struct BitRange { - size_t start; - size_t end; + slang::bitwidth_t start; + slang::bitwidth_t end; BitRange(size_t index) : start(index), end(index) {} BitRange(size_t start, size_t end) : start(start), end(end) { SLANG_ASSERT(start <= end); } - /// Given two ranges, return true if there is any overlap in values between them. + /// Given another range, return true if it overlaps with this range. inline bool overlap(BitRange other) const { return start <= other.end && other.start <= end; } + /// Given another range, return true if it is contained within this range. + inline bool contains(BitRange other) { + return other.start >= start && other.end <= end; + } size_t size() const { return end - start; } }; @@ -68,20 +72,74 @@ class AnalyseVariableReference { BitRange handleScalarRangeSelect(const slang::ast::Type &type, BitRange range) { const auto& rangeSelector = selectorsIt->get()->as(); - SLANG_ASSERT(rangeSelector.getRightIndexInt() >= range.start); - SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= range.end); - SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rangeSelector.getLeftIndexInt()); + slang::bitwidth_t leftIndex = rangeSelector.getLeftIndexInt(); + slang::bitwidth_t rightIndex = rangeSelector.getRightIndexInt(); + SLANG_ASSERT(rightIndex <= leftIndex); + SLANG_ASSERT(rightIndex >= range.start); + SLANG_ASSERT(leftIndex <= range.end); + auto newRange = BitRange(range.start + rightIndex, + range.start + leftIndex); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + return getBitRangeImpl(type, newRange); + } else { + return newRange; + } + } + + BitRange handleArrayElementSelect(const slang::ast::Type &type, BitRange range) { + const auto& elementSelector = selectorsIt->get()->as(); + slang::bitwidth_t index = elementSelector.getIndexInt(); + auto* elementType = type.getArrayElementType(); + SLANG_ASSERT((index * elementType->getBitWidth()) >= range.start); + SLANG_ASSERT(((index + 1) * elementType->getBitWidth()) <= range.end); + auto newRange = BitRange(range.start + (elementType->getBitWidth() * index), + range.start + (elementType->getBitWidth() * (index + 1))); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + return getBitRangeImpl(*elementType, newRange); + } else { + return newRange; + } + } + + BitRange handleArrayRangeSelect(const slang::ast::Type &type, BitRange range) { + const auto& rangeSelector = selectorsIt->get()->as(); + slang::bitwidth_t leftIndex = rangeSelector.getLeftIndexInt(); + slang::bitwidth_t rightIndex = rangeSelector.getRightIndexInt(); + auto* elementType = type.getArrayElementType(); + SLANG_ASSERT(rightIndex <= leftIndex); + SLANG_ASSERT((rightIndex * elementType->getBitWidth()) >= range.start); + SLANG_ASSERT((leftIndex * elementType->getBitWidth()) <= range.end); + auto newRange = BitRange(range.start + (rightIndex * elementType->getBitWidth()), + range.start + (leftIndex * elementType->getBitWidth())); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; - return getBitRangeImpl(type, - BitRange(rangeSelector.getRightIndexInt(), - rangeSelector.getLeftIndexInt())); + return getBitRangeImpl(type, newRange); } else { - return BitRange(rangeSelector.getRightIndexInt(), - rangeSelector.getLeftIndexInt()); + return newRange; } } + BitRange handleStructMemberAccess(const slang::ast::Type &type, BitRange range) { + //const auto& packedStruct = type.getCanonicalType().as(); + //SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); + //const auto& memberAccessSelector = selectorsIt->get()->as(); + //auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); + //SLANG_ASSERT(range.contains(fieldRange)); + //auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; + //return getBitRange(fieldType, fieldRange.start, fieldRange.end); + return BitRange(0); + } + + BitRange handleUnionMemberAccess(const slang::ast::Type &type, BitRange range) { + return BitRange(0); + } + + BitRange handleEnumMemberAccess(const slang::ast::Type &type, BitRange range) { + return BitRange(0); + } + /// Given a variable reference with zero or more selectors, determine the /// bit range that is accessed. BitRange getBitRangeImpl(const slang::ast::Type &type, BitRange range) { @@ -89,7 +147,7 @@ class AnalyseVariableReference { if (node.selectors.empty()) { return BitRange(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); } - // Scalar + // Simple vector if (type.isSimpleBitVector()) { if (selectorsIt->get()->isElementSelect()) { return handleScalarElementSelect(type, range); @@ -101,51 +159,51 @@ class AnalyseVariableReference { SLANG_ASSERT(0 && "unsupported scalar selector"); } } - //// Packed or unpacked array - //else if (type.isArray()) { - // if (selectorsIt->get()->kind == VariableSelectorKind::ElementSelect) { - // const auto& elementSelector = selectorsIt->get()->as(); - // size_t index = leftIndex + elementSelector.getIndexInt(); - // // TODO: also needs to recurse if there are more selectors - // return std::make_pair(index, index); - // } - // else if (selectorsIt->get()->kind == VariableSelectorKind::RangeSelect) { - // const auto& rangeSelector = selectorsIt->get()->as(); - // SLANG_ASSERT(rangeSelector.getLeftIndexInt() <= rightIndex); - // SLANG_ASSERT(rangeSelector.getRightIndexInt() <= rightIndex); - // SLANG_ASSERT(rangeSelector.getRightIndexInt() - rangeSelector.getLeftIndexInt() < rightIndex - leftIndex); - // if (std::next(selectorsIt) != node.selectors.end()) { - // return getBitRange(node, type, std::next(selectorsIt), - // leftIndex + rangeSelector.getLeftIndexInt(), - // leftIndex + rangeSelector.getRightIndexInt()); - // } else { - // return std::make_pair(leftIndex + rangeSelector.getLeftIndexInt(), - // leftIndex + rangeSelector.getRightIndexInt()); - // } - // } - // else { - // SLANG_ASSERT(0 && "unsupported array selector"); - // } - //} - //// Packed struct - //else if (type.isStruct() && !type.isUnpackedStruct()) { - // const auto& packedStruct = type.getCanonicalType().as(); - // SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); - // const auto& memberAccessSelector = selectorsIt->get()->as(); - // auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); - // SLANG_ASSERT(fieldRange.first >= leftIndex); - // SLANG_ASSERT(fieldRange.second <= rightIndex); - // auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; - // return getBitRange(node, fieldType, std::next(selectorsIt), - // leftIndex + fieldRange.first, - // leftIndex + fieldRange.second); - //} - //else if (type.isPackedUnion()) { - //} - //else if (type.isEnum()) { - //} + // Packed or unpacked array + else if (type.isArray()) { + if (std::next(selectorsIt) != node.selectors.end() && + (std::next(selectorsIt)->get()->isElementSelect() || std::next(selectorsIt)->get()->isRangeSelect())) { + // Multiple range or element selectors have only the effect of + // the last one. + selectorsIt++; + return getBitRangeImpl(type, range); + } + if (selectorsIt->get()->isElementSelect()) { + return handleArrayElementSelect(type, range); + } + else if (selectorsIt->get()->isRangeSelect()) { + return handleArrayRangeSelect(type, range); + } + else { + SLANG_ASSERT(0 && "unsupported array selector"); + } + } + // Packed struct + else if (type.isStruct() && !type.isUnpackedStruct()) { + if (selectorsIt->get()->isMemberAccess()) { + return handleStructMemberAccess(type, range); + } else { + SLANG_ASSERT(0 && "unsupported struct selector"); + } + } + // Packed union + else if (type.isPackedUnion()) { + if (selectorsIt->get()->isMemberAccess()) { + return handleUnionMemberAccess(type, range); + } else { + SLANG_ASSERT(0 && "unsupported union selector"); + } + } + // Enum + else if (type.isEnum()) { + if (selectorsIt->get()->isMemberAccess()) { + return handleEnumMemberAccess(type, range); + } else { + SLANG_ASSERT(0 && "unsupported enum selector"); + } + } else { - SLANG_THROW(std::runtime_error("unhandled type")); + SLANG_ASSERT(0 && "unhandled type in getBitRangeImpl"); } } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 84d1a5ebc..2ce13bc1a 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -65,7 +65,7 @@ module m (input logic [3:0] i_a, input logic [3:0] i_b, output logic [1:0] o_a, output logic [1:0] o_b); - int foo, bar; + int foo; assign foo[7:4] = i_a; assign foo[3:0] = i_b; assign o_a = foo[7:4][6:5]; @@ -83,6 +83,106 @@ endmodule CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); } +TEST_CASE("Packed array element") { + // Test element select on a packed array variable. + auto tree = SyntaxTree::fromText(R"( +module m (input logic i_a, + input logic i_b, + output logic o_a, + output logic o_b); + logic [1:0] foo; + assign foo[1] = i_a; + assign foo[0] = i_b; + assign o_a = foo[1]; + assign o_b = foo[0]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(pathExists(netlist, "m.i_a", "m.o_a")); + CHECK(pathExists(netlist, "m.i_b", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +} + +TEST_CASE("Unpacked array element") { + // Test element select on an unpacked array variable. + auto tree = SyntaxTree::fromText(R"( +module m (input logic i_a, + input logic i_b, + output logic o_a, + output logic o_b); + logic foo [1:0]; + assign foo[1] = i_a; + assign foo[0] = i_b; + assign o_a = foo[1]; + assign o_b = foo[0]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(pathExists(netlist, "m.i_a", "m.o_a")); + CHECK(pathExists(netlist, "m.i_b", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +} + + +TEST_CASE("Packed array range") { + // Test range select on a packed array variable. + auto tree = SyntaxTree::fromText(R"( +module m (input logic [1:0] i_a, + input logic [1:0] i_b, + output logic [1:0] o_a, + output logic [1:0] o_b); + logic [3:0] foo; + assign foo[3:2] = i_a; + assign foo[1:0] = i_b; + assign o_a = foo[3:2]; + assign o_b = foo[1:0]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + CHECK(pathExists(netlist, "m.i_a", "m.o_a")); + CHECK(pathExists(netlist, "m.i_b", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +} + +TEST_CASE("Packed array range 2") { + // Test stacked range selects on a packed array variable. + auto tree = SyntaxTree::fromText(R"( +module m (input logic [3:0] i_a, + input logic [3:0] i_b, + output logic [1:0] o_a, + output logic [1:0] o_b); + logic [7:0] foo; + assign foo[7:4] = i_a; + assign foo[3:0] = i_b; + assign o_a = foo[7:4][6:5]; + assign o_b = foo[3:0][2:1]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + CHECK(pathExists(netlist, "m.i_a", "m.o_a")); + CHECK(pathExists(netlist, "m.i_b", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +} + + //===---------------------------------------------------------------------===// // Tests for variable splitting From 7e6c3e709635c266d0e147898634c74913c65e58 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Mon, 14 Aug 2023 21:22:03 +0100 Subject: [PATCH 05/31] Fix unit test failure --- tools/netlist/include/SplitVariables.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 1c0c2c833..3b22e80e0 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -94,7 +94,7 @@ class AnalyseVariableReference { SLANG_ASSERT((index * elementType->getBitWidth()) >= range.start); SLANG_ASSERT(((index + 1) * elementType->getBitWidth()) <= range.end); auto newRange = BitRange(range.start + (elementType->getBitWidth() * index), - range.start + (elementType->getBitWidth() * (index + 1))); + range.start + (elementType->getBitWidth() * (index + 1)) - 1); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); @@ -112,7 +112,7 @@ class AnalyseVariableReference { SLANG_ASSERT((rightIndex * elementType->getBitWidth()) >= range.start); SLANG_ASSERT((leftIndex * elementType->getBitWidth()) <= range.end); auto newRange = BitRange(range.start + (rightIndex * elementType->getBitWidth()), - range.start + (leftIndex * elementType->getBitWidth())); + range.start + (leftIndex * elementType->getBitWidth()) - 1); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -162,9 +162,11 @@ class AnalyseVariableReference { // Packed or unpacked array else if (type.isArray()) { if (std::next(selectorsIt) != node.selectors.end() && - (std::next(selectorsIt)->get()->isElementSelect() || std::next(selectorsIt)->get()->isRangeSelect())) { + (std::next(selectorsIt)->get()->isElementSelect() || + std::next(selectorsIt)->get()->isRangeSelect())) { // Multiple range or element selectors have only the effect of - // the last one. + // the last one. Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> + // x[2]. selectorsIt++; return getBitRangeImpl(type, range); } From 5b265da11d3badff43804895d1ae27ca84b3171d Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Tue, 15 Aug 2023 22:22:16 +0100 Subject: [PATCH 06/31] WIP - Handle non-zero array indexing - Handle all arrays with array logic - Add SelectionKind to range select to prepare for handling of different range operators - Prepare for handling of non-constant elment and range selects. --- tools/netlist/TODO.md | 4 +- tools/netlist/include/Netlist.h | 10 ++- tools/netlist/include/NetlistVisitor.h | 2 +- tools/netlist/include/SplitVariables.h | 46 ++++++++--- .../netlist/tests/VariableSelectorsTests.cpp | 78 +++++++++++++------ 5 files changed, 98 insertions(+), 42 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index 40f53b68c..a952424ce 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -39,8 +39,10 @@ To dos - In SplitVariables, handle: * Nested range selections, eg `x[3:0][2:0][1:0]` has a dependency with `x[0]`, so effectively multiple ranges should be flattened eg to `[1:0]`. - * Unions, where they casue access cannot to no longer be tracked through a + * Unions, where they cause access cannot to no longer be tracked through a heirarchy of types. Instead access must be resolved on the bit level (bit blasted). + * Variable positions in element or range selects. Note that in [x+:y] y must be a + constant. - Dumping of a dot file outputs random characters at the end. - Reporting of variables in the netlist (by type, matching patterns). diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 74c3797f7..8416c43c3 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -15,6 +15,7 @@ #include #include "slang/ast/ASTVisitor.h" +#include "slang/ast/Expression.h" #include "slang/ast/symbols/CompilationUnitSymbols.h" #include "slang/diagnostics/TextDiagnosticClient.h" #include "slang/syntax/SyntaxTree.h" @@ -86,10 +87,11 @@ struct VariableElementSelect : public VariableSelectorBase { /// A variable selector representing a range selector. struct VariableRangeSelect : public VariableSelectorBase { ConstantValue leftIndex, rightIndex; + ast::RangeSelectionKind selectionKind; - VariableRangeSelect(ConstantValue leftIndex, ConstantValue rightIndex) : + VariableRangeSelect(ConstantValue leftIndex, ConstantValue rightIndex, ast::RangeSelectionKind selectionKind) : VariableSelectorBase(VariableSelectorKind::RangeSelect), leftIndex(std::move(leftIndex)), - rightIndex(std::move(rightIndex)) {} + rightIndex(std::move(rightIndex)), selectionKind(selectionKind) {} static bool isKind(VariableSelectorKind otherKind) { return otherKind == VariableSelectorKind::RangeSelect; @@ -229,8 +231,8 @@ class NetlistVariableReference : public NetlistNode { void addElementSelect(const ConstantValue& index) { selectors.emplace_back(std::make_unique(index)); } - void addRangeSelect(const ConstantValue& leftIndex, const ConstantValue& rightIndex) { - selectors.emplace_back(std::make_unique(leftIndex, rightIndex)); + void addRangeSelect(const ConstantValue& leftIndex, const ConstantValue& rightIndex, ast::RangeSelectionKind selectionKind) { + selectors.emplace_back(std::make_unique(leftIndex, rightIndex, selectionKind)); } void addMemberAccess(std::string_view name) { selectors.emplace_back(std::make_unique(name)); diff --git a/tools/netlist/include/NetlistVisitor.h b/tools/netlist/include/NetlistVisitor.h index 878b1ad95..65d94d913 100644 --- a/tools/netlist/include/NetlistVisitor.h +++ b/tools/netlist/include/NetlistVisitor.h @@ -80,7 +80,7 @@ class VariableReferenceVisitor : public ast::ASTVisitoras(); auto leftIndex = rangeSelectExpr.left().eval(evalCtx); auto rightIndex = rangeSelectExpr.right().eval(evalCtx); - node.addRangeSelect(leftIndex, rightIndex); + node.addRangeSelect(leftIndex, rightIndex, rangeSelectExpr.getSelectionKind()); } else if (selector->kind == ast::ExpressionKind::MemberAccess) { node.addMemberAccess(selector->as().member.name); diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 3b22e80e0..dd4f9c969 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -13,8 +13,10 @@ #include "fmt/format.h" #include +#include "slang/ast/Symbol.h" #include "slang/ast/types/AllTypes.h" #include "slang/ast/types/Type.h" +#include "slang/numeric/SVInt.h" #include "slang/util/Util.h" @@ -62,6 +64,21 @@ class AnalyseVariableReference { SLANG_UNREACHABLE; } + /// Given an array type, return the base from which this array is indexed. + const slang::ConstantRange& getArrayRange(const slang::ast::Type &type) { + if (type.kind == slang::ast::SymbolKind::PackedArrayType) { + auto& arrayType = type.as(); + return arrayType.range; + } + else if (type.kind == slang::ast::SymbolKind::FixedSizeUnpackedArrayType) { + auto& arrayType = type.as(); + return arrayType.range; + } + else { + SLANG_ASSERT(0 && "unexpected array type"); + } + } + BitRange handleScalarElementSelect(const slang::ast::Type &type, BitRange range) { const auto& elementSelector = selectorsIt->get()->as(); SLANG_ASSERT(elementSelector.getIndexInt() >= range.start); @@ -89,12 +106,15 @@ class AnalyseVariableReference { BitRange handleArrayElementSelect(const slang::ast::Type &type, BitRange range) { const auto& elementSelector = selectorsIt->get()->as(); - slang::bitwidth_t index = elementSelector.getIndexInt(); + size_t index = elementSelector.getIndexInt(); + auto& arrayRange = getArrayRange(type); + SLANG_ASSERT(index >= arrayRange.right); + SLANG_ASSERT(index <= arrayRange.left); + // Adjust for non-zero array indexing. + index -= arrayRange.right; auto* elementType = type.getArrayElementType(); - SLANG_ASSERT((index * elementType->getBitWidth()) >= range.start); - SLANG_ASSERT(((index + 1) * elementType->getBitWidth()) <= range.end); - auto newRange = BitRange(range.start + (elementType->getBitWidth() * index), - range.start + (elementType->getBitWidth() * (index + 1)) - 1); + auto newRange = BitRange(range.start + (index * elementType->getBitWidth()), + range.start + ((index + 1) * elementType->getBitWidth()) - 1); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); @@ -105,12 +125,16 @@ class AnalyseVariableReference { BitRange handleArrayRangeSelect(const slang::ast::Type &type, BitRange range) { const auto& rangeSelector = selectorsIt->get()->as(); - slang::bitwidth_t leftIndex = rangeSelector.getLeftIndexInt(); - slang::bitwidth_t rightIndex = rangeSelector.getRightIndexInt(); - auto* elementType = type.getArrayElementType(); + size_t leftIndex = rangeSelector.getLeftIndexInt(); + size_t rightIndex = rangeSelector.getRightIndexInt(); + auto& arrayRange = getArrayRange(type); + SLANG_ASSERT(rightIndex >= arrayRange.right); + SLANG_ASSERT(leftIndex <= arrayRange.left); SLANG_ASSERT(rightIndex <= leftIndex); - SLANG_ASSERT((rightIndex * elementType->getBitWidth()) >= range.start); - SLANG_ASSERT((leftIndex * elementType->getBitWidth()) <= range.end); + // Adjust for non-zero array indexing. + leftIndex -= arrayRange.right; + rightIndex -= arrayRange.right; + auto* elementType = type.getArrayElementType(); auto newRange = BitRange(range.start + (rightIndex * elementType->getBitWidth()), range.start + (leftIndex * elementType->getBitWidth()) - 1); if (std::next(selectorsIt) != node.selectors.end()) { @@ -148,7 +172,7 @@ class AnalyseVariableReference { return BitRange(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); } // Simple vector - if (type.isSimpleBitVector()) { + if (type.isPredefinedInteger() || type.isScalar()) { if (selectorsIt->get()->isElementSelect()) { return handleScalarElementSelect(type, range); } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 2ce13bc1a..d88290cc1 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -33,6 +33,33 @@ endmodule CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); } +//TEST_CASE("Scalar variable element") { +// // Test variable element select on a scalar variable. +// auto tree = SyntaxTree::fromText(R"( +//module m (input logic i_a, +// input logic i_b, +// input logic i_sel, +// output logic o_a, +// output logic o_b); +// int foo; +// always_comb begin +// foo[i_sel] = i_a; +// foo[!i_sel] = i_b; +// end +// assign o_a = foo[i_sel]; +// assign o_b = foo[!i_sel]; +//endmodule +//)"); +// Compilation compilation; +// compilation.addSyntaxTree(tree); +// NO_COMPILATION_ERRORS; +// auto netlist = createNetlist(compilation); +// CHECK(pathExists(netlist, "m.i_a", "m.o_a")); +// CHECK(pathExists(netlist, "m.i_b", "m.o_b")); +// CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); +// CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +//} + TEST_CASE("Scalar range") { // Test range select on a scalar variable. auto tree = SyntaxTree::fromText(R"( @@ -107,31 +134,6 @@ endmodule CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); } -TEST_CASE("Unpacked array element") { - // Test element select on an unpacked array variable. - auto tree = SyntaxTree::fromText(R"( -module m (input logic i_a, - input logic i_b, - output logic o_a, - output logic o_b); - logic foo [1:0]; - assign foo[1] = i_a; - assign foo[0] = i_b; - assign o_a = foo[1]; - assign o_b = foo[0]; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); -} - - TEST_CASE("Packed array range") { // Test range select on a packed array variable. auto tree = SyntaxTree::fromText(R"( @@ -182,6 +184,32 @@ endmodule CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); } +TEST_CASE("Unpacked array element") { + // Test element select on an unpacked array variable. + auto tree = SyntaxTree::fromText(R"( +module m (input logic i_a, + input logic i_b, + output logic o_a, + output logic o_b); + logic foo [1:0]; + assign foo[1] = i_a; + assign foo[0] = i_b; + assign o_a = foo[1]; + assign o_b = foo[0]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(pathExists(netlist, "m.i_a", "m.o_a")); + CHECK(pathExists(netlist, "m.i_b", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); + CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +} + +// TODO: variable positions in element and range selects +// TODO: [x:y], [x+:y] and [x-:y] range selects //===---------------------------------------------------------------------===// From 353fda8a6d2d9e7d018e03123f13723e147bafbe Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 18 Aug 2023 10:14:54 +0100 Subject: [PATCH 07/31] WIP refactoring unit tests --- tools/netlist/TODO.md | 10 +- tools/netlist/include/Netlist.h | 32 +- tools/netlist/include/SplitVariables.h | 28 +- .../netlist/tests/VariableSelectorsTests.cpp | 289 +++++++----------- 4 files changed, 167 insertions(+), 192 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index a952424ce..6cc223fcb 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -37,12 +37,14 @@ To dos ProceduralCheckerStatement - In SplitVariables, handle: - * Nested range selections, eg `x[3:0][2:0][1:0]` has a dependency with - `x[0]`, so effectively multiple ranges should be flattened eg to `[1:0]`. * Unions, where they cause access cannot to no longer be tracked through a heirarchy of types. Instead access must be resolved on the bit level (bit blasted). - * Variable positions in element or range selects. Note that in [x+:y] y must be a - constant. + * Variable positions in element or range selects [x:y]. + Note that in [x+:y] y must be a constant. + Also handle [x-:y]. + +- Optimise lookups of nodes in the netlist by adding tables for variable + declarations, variable references, ports etc. - Dumping of a dot file outputs random characters at the end. - Reporting of variables in the netlist (by type, matching patterns). diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 8416c43c3..771becb1c 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -50,6 +50,7 @@ struct VariableSelectorBase { bool isElementSelect() const { return kind == VariableSelectorKind::ElementSelect; } bool isRangeSelect() const { return kind == VariableSelectorKind::RangeSelect; } bool isMemberAccess() const { return kind == VariableSelectorKind::MemberAccess; } + bool isArraySelect() const { return isElementSelect() || isRangeSelect(); } template T& as() { @@ -314,26 +315,37 @@ class Netlist : public DirectedGraph { return node; } + /// Find a port declaration node in the netlist by hierarchical path. + NetlistPortDeclaration* lookupPort(std::string_view hierarchicalPath) { + auto compareNode = [&hierarchicalPath](const std::unique_ptr& node) { + return node->kind == NodeKind::PortDeclaration && + node->as().hierarchicalPath == hierarchicalPath; + }; + auto it = std::ranges::find_if(*this, compareNode); + return it != end() ? &it->get()->as() : nullptr; + } + /// Find a variable declaration node in the netlist by hierarchical path. - /// TODO? Optimise this lookup by maintaining a list of declaration nodes. - NetlistNode* lookupVariable(const std::string& hierarchicalPath) { + /// Note that this does not lookup alias nodes. + NetlistVariableDeclaration* lookupVariable(std::string_view hierarchicalPath) { auto compareNode = [&hierarchicalPath](const std::unique_ptr& node) { return node->kind == NodeKind::VariableDeclaration && node->as().hierarchicalPath == hierarchicalPath; }; auto it = std::ranges::find_if(*this, compareNode); - return it != end() ? it->get() : nullptr; + return it != end() ? &it->get()->as() : nullptr; } - /// Find a port declaration node in the netlist by hierarchical path. - /// TODO? Optimise this lookup by maintaining a list of port nodes. - NetlistNode* lookupPort(const std::string& hierarchicalPath) { - auto compareNode = [&hierarchicalPath](const std::unique_ptr& node) { - return node->kind == NodeKind::PortDeclaration && - node->as().hierarchicalPath == hierarchicalPath; + /// Find a variable reference node in the netlist by its syntax. + /// Note that this does not include the hierarchical path, which is only + /// associated with the corresponding variable declaration nodes. + NetlistVariableReference* lookupVariableReference(std::string_view syntax) { + auto compareNode = [&syntax](const std::unique_ptr& node) { + return node->kind == NodeKind::VariableReference && + node->as().toString() == syntax; }; auto it = std::ranges::find_if(*this, compareNode); - return it != end() ? it->get() : nullptr; + return it != end() ? &it->get()->as() : nullptr; } }; diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index dd4f9c969..4aff6abff 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -11,6 +11,7 @@ #include "Netlist.h" #include "fmt/color.h" #include "fmt/format.h" +#include #include #include "slang/ast/Symbol.h" @@ -34,10 +35,20 @@ struct BitRange { return start <= other.end && other.start <= end; } /// Given another range, return true if it is contained within this range. - inline bool contains(BitRange other) { + inline bool contains(BitRange other) const { return other.start >= start && other.end <= end; } + /// Equality. + friend bool operator==(BitRange const & A, BitRange const & B) noexcept { + return A.start == B.start && A.end == B.end; + } + // Output to stream. + friend std::ostream& operator<<(std::ostream& os, const BitRange& range) { + os << fmt::format("BitRange {}", range.toString()); + return os; + } size_t size() const { return end - start; } + std::string toString() const { return fmt::format("[{}:{}]", end, start); } }; class AnalyseVariableReference { @@ -46,10 +57,13 @@ class AnalyseVariableReference { NetlistVariableReference::SelectorsListType::iterator selectorsIt; public: - AnalyseVariableReference(NetlistVariableReference &node) : - node(node), selectorsIt(node.selectors.begin()) { + static AnalyseVariableReference create(NetlistVariableReference &node) { + return AnalyseVariableReference(node); } + AnalyseVariableReference(NetlistVariableReference &node) : + node(node), selectorsIt(node.selectors.begin()) {} + /// Given a packed struct type, return the bit position of the named field. BitRange getFieldRange(const slang::ast::PackedStructType &packedStruct, const std::string_view fieldName) const { @@ -64,7 +78,7 @@ class AnalyseVariableReference { SLANG_UNREACHABLE; } - /// Given an array type, return the base from which this array is indexed. + /// Given an array type, return the range from which the array is indexed. const slang::ConstantRange& getArrayRange(const slang::ast::Type &type) { if (type.kind == slang::ast::SymbolKind::PackedArrayType) { auto& arrayType = type.as(); @@ -186,9 +200,9 @@ class AnalyseVariableReference { // Packed or unpacked array else if (type.isArray()) { if (std::next(selectorsIt) != node.selectors.end() && - (std::next(selectorsIt)->get()->isElementSelect() || - std::next(selectorsIt)->get()->isRangeSelect())) { - // Multiple range or element selectors have only the effect of + (selectorsIt->get()->isRangeSelect() && + std::next(selectorsIt)->get()->isArraySelect())) { + // Multiple range selectors have only the effect of // the last one. Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> // x[2]. selectorsIt++; diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index d88290cc1..ff1ac252b 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -7,210 +7,157 @@ //------------------------------------------------------------------------------ #include "NetlistTest.h" +#include "SplitVariables.h" -TEST_CASE("Scalar element") { - // Test element select on a scalar variable. - auto tree = SyntaxTree::fromText(R"( -module m (input logic i_a, - input logic i_b, - output logic o_a, - output logic o_b); - int foo; - assign foo[1] = i_a; - assign foo[0] = i_b; - assign o_a = foo[1]; - assign o_b = foo[0]; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); -} - -//TEST_CASE("Scalar variable element") { -// // Test variable element select on a scalar variable. -// auto tree = SyntaxTree::fromText(R"( -//module m (input logic i_a, -// input logic i_b, -// input logic i_sel, -// output logic o_a, -// output logic o_b); -// int foo; -// always_comb begin -// foo[i_sel] = i_a; -// foo[!i_sel] = i_b; -// end -// assign o_a = foo[i_sel]; -// assign o_b = foo[!i_sel]; -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -// CHECK(pathExists(netlist, "m.i_a", "m.o_a")); -// CHECK(pathExists(netlist, "m.i_b", "m.o_b")); -// CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); -// CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); -//} - -TEST_CASE("Scalar range") { - // Test range select on a scalar variable. - auto tree = SyntaxTree::fromText(R"( -module m (input logic [1:0] i_a, - input logic [1:0] i_b, - output logic [1:0] o_a, - output logic [1:0] o_b); - int foo; - assign foo[3:2] = i_a; - assign foo[1:0] = i_b; - assign o_a = foo[3:2]; - assign o_b = foo[1:0]; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); +/// Helper method to extract a variable reference from a netlist and return the +/// bit range associated with it. +BitRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { + auto* node = netlist.lookupVariableReference(variableSyntax); + return AnalyseVariableReference::create(*node).getBitRange(); } -TEST_CASE("Scalar range 2") { - // Test stacked range selects on a scalar variable. +TEST_CASE("Scalar element and range") { + // Test element select on a scalar variable. auto tree = SyntaxTree::fromText(R"( -module m (input logic [3:0] i_a, - input logic [3:0] i_b, - output logic [1:0] o_a, - output logic [1:0] o_b); +module m; int foo; - assign foo[7:4] = i_a; - assign foo[3:0] = i_b; - assign o_a = foo[7:4][6:5]; - assign o_b = foo[3:0][2:1]; + always_comb begin + foo[0] = 0; + foo[1] = 0; + foo[7:7] = 0; + foo[1:0] = 0; + foo[3:1] = 0; + foo[7:4] = 0; + foo[3:1][2:1] = 0; + foo[7:4][6:5] = 0; + foo[3:1][2:1][1] = 0; + foo[7:4][6:5][5] = 0; + end endmodule )"); Compilation compilation; compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); + CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); + CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(7)); + CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[3:1]") == BitRange(1, 3)); + CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(4, 7)); + CHECK(getBitRange(netlist, "foo[7:4][6:5]") == BitRange(5, 6)); + CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(5)); } -TEST_CASE("Packed array element") { - // Test element select on a packed array variable. +TEST_CASE("Packed 1D array element and range") { auto tree = SyntaxTree::fromText(R"( -module m (input logic i_a, - input logic i_b, - output logic o_a, - output logic o_b); - logic [1:0] foo; - assign foo[1] = i_a; - assign foo[0] = i_b; - assign o_a = foo[1]; - assign o_b = foo[0]; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); -} - -TEST_CASE("Packed array range") { - // Test range select on a packed array variable. - auto tree = SyntaxTree::fromText(R"( -module m (input logic [1:0] i_a, - input logic [1:0] i_b, - output logic [1:0] o_a, - output logic [1:0] o_b); +module m; logic [3:0] foo; - assign foo[3:2] = i_a; - assign foo[1:0] = i_b; - assign o_a = foo[3:2]; - assign o_b = foo[1:0]; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); -} - -TEST_CASE("Packed array range 2") { - // Test stacked range selects on a packed array variable. - auto tree = SyntaxTree::fromText(R"( -module m (input logic [3:0] i_a, - input logic [3:0] i_b, - output logic [1:0] o_a, - output logic [1:0] o_b); - logic [7:0] foo; - assign foo[7:4] = i_a; - assign foo[3:0] = i_b; - assign o_a = foo[7:4][6:5]; - assign o_b = foo[3:0][2:1]; + always_comb begin + foo[0] = 0; + foo[1] = 0; + foo[2] = 0; + foo[3] = 0; + foo[3:3] = 0; + foo[1:0] = 0; + foo[3:0] = 0; + foo[2:1] = 0; + end endmodule )"); Compilation compilation; compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); + CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); + CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[2]") == BitRange(2)); + CHECK(getBitRange(netlist, "foo[3]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[3:3]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[3:0]") == BitRange(0, 3)); + CHECK(getBitRange(netlist, "foo[2:1]") == BitRange(1, 2)); } -TEST_CASE("Unpacked array element") { - // Test element select on an unpacked array variable. +TEST_CASE("Packed 1D array element and range non-zero indexed") { auto tree = SyntaxTree::fromText(R"( -module m (input logic i_a, - input logic i_b, - output logic o_a, - output logic o_b); - logic foo [1:0]; - assign foo[1] = i_a; - assign foo[0] = i_b; - assign o_a = foo[1]; - assign o_b = foo[0]; +module m; + logic [7:4] foo; + always_comb begin + foo[4] = 0; + foo[5] = 0; + foo[6] = 0; + foo[7] = 0; + foo[7:7] = 0; + foo[5:4] = 0; + foo[7:4] = 0; + foo[6:5] = 0; + end endmodule )"); Compilation compilation; compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(pathExists(netlist, "m.i_a", "m.o_a")); - CHECK(pathExists(netlist, "m.i_b", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_a", "m.o_b")); - CHECK(!pathExists(netlist, "m.i_b", "m.o_a")); + CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); + CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[2]") == BitRange(2)); + CHECK(getBitRange(netlist, "foo[3]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[3:0]") == BitRange(0, 3)); + CHECK(getBitRange(netlist, "foo[2:1]") == BitRange(1, 2)); } -// TODO: variable positions in element and range selects -// TODO: [x:y], [x+:y] and [x-:y] range selects - +//TEST_CASE("Packed 2D array element and range") { +// // Test element select on a packed array variable. +// auto tree = SyntaxTree::fromText(R"( +//module m; +// logic [1:0] [1:0] foo; +// always_comb begin +// foo[0] = 0; +// foo[1] = 0; +// foo[0][0] = 0; +// foo[0][1] = 0; +// foo[1][0] = 0; +// foo[1][1] = 0; +// assign foo[3:2] = i_a; +// assign foo[1:0] = i_b; +// assign o_a = foo[7:4][6:5]; +// assign o_b = foo[3:0][2:1]; +// end +//endmodule +//)"); +// Compilation compilation; +// compilation.addSyntaxTree(tree); +// NO_COMPILATION_ERRORS; +// auto netlist = createNetlist(compilation); +// CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); +// CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); +// CHECK(getBitRange(netlist, "foo[0][0]") == BitRange(0)); +// CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); +// CHECK(getBitRange(netlist, "foo[1][0]") == BitRange(2)); +// CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); +//} +// +//TEST_CASE("Unpacked 1D array element") { +// // Test element select on an unpacked array variable. +// auto tree = SyntaxTree::fromText(R"( +//module m; +// logic foo [1:0]; +// always_comb begin +// foo[0] = 0; +// foo[1] = 0; +// end +//endmodule +//)"); +// Compilation compilation; +// compilation.addSyntaxTree(tree); +// NO_COMPILATION_ERRORS; +// auto netlist = createNetlist(compilation); +//} //===---------------------------------------------------------------------===// // Tests for variable splitting From 3e552db0eaec244dc927d415d64b1f3fa5efea1b Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 18 Aug 2023 10:24:53 +0100 Subject: [PATCH 08/31] Split BitRange into file --- tools/netlist/include/BitRange.h | 45 +++++++++++++++++++ tools/netlist/include/SplitVariables.h | 31 +------------ .../netlist/tests/VariableSelectorsTests.cpp | 14 +++--- 3 files changed, 53 insertions(+), 37 deletions(-) create mode 100644 tools/netlist/include/BitRange.h diff --git a/tools/netlist/include/BitRange.h b/tools/netlist/include/BitRange.h new file mode 100644 index 000000000..568440f43 --- /dev/null +++ b/tools/netlist/include/BitRange.h @@ -0,0 +1,45 @@ +//------------------------------------------------------------------------------ +//! @file BitRange.h +//! @brieif A class to represent a bit range. +// +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT +//------------------------------------------------------------------------------ +#pragma once + +#include +#include +#include "fmt/format.h" + +#include "slang/numeric/SVInt.h" + + +struct BitRange { + slang::bitwidth_t start; + slang::bitwidth_t end; + BitRange(size_t index) : start(index), end(index) {} + BitRange(size_t start, size_t end) : start(start), end(end) { + SLANG_ASSERT(start <= end); + } + /// Given another range, return true if it overlaps with this range. + inline bool overlap(BitRange other) const { + return start <= other.end && other.start <= end; + } + /// Given another range, return true if it is contained within this range. + inline bool contains(BitRange other) const { + return other.start >= start && other.end <= end; + } + /// Equality. + friend bool operator==(BitRange const & A, BitRange const & B) noexcept { + return A.start == B.start && A.end == B.end; + } + // Output to stream. + friend std::ostream& operator<<(std::ostream& os, const BitRange& range) { + os << fmt::format("BitRange {}", range.toString()); + return os; + } + size_t size() const { return end - start; } + std::string toString() const { return fmt::format("[{}:{}]", end, start); } +}; + + diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 4aff6abff..0d3de4557 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -11,7 +11,6 @@ #include "Netlist.h" #include "fmt/color.h" #include "fmt/format.h" -#include #include #include "slang/ast/Symbol.h" @@ -19,38 +18,10 @@ #include "slang/ast/types/Type.h" #include "slang/numeric/SVInt.h" #include "slang/util/Util.h" - +#include "BitRange.h" namespace netlist { -struct BitRange { - slang::bitwidth_t start; - slang::bitwidth_t end; - BitRange(size_t index) : start(index), end(index) {} - BitRange(size_t start, size_t end) : start(start), end(end) { - SLANG_ASSERT(start <= end); - } - /// Given another range, return true if it overlaps with this range. - inline bool overlap(BitRange other) const { - return start <= other.end && other.start <= end; - } - /// Given another range, return true if it is contained within this range. - inline bool contains(BitRange other) const { - return other.start >= start && other.end <= end; - } - /// Equality. - friend bool operator==(BitRange const & A, BitRange const & B) noexcept { - return A.start == B.start && A.end == B.end; - } - // Output to stream. - friend std::ostream& operator<<(std::ostream& os, const BitRange& range) { - os << fmt::format("BitRange {}", range.toString()); - return os; - } - size_t size() const { return end - start; } - std::string toString() const { return fmt::format("[{}:{}]", end, start); } -}; - class AnalyseVariableReference { private: NetlistVariableReference& node; diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index ff1ac252b..7216135c4 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -101,14 +101,14 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); - CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[2]") == BitRange(2)); - CHECK(getBitRange(netlist, "foo[3]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[4]") == BitRange(0)); + CHECK(getBitRange(netlist, "foo[5]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[6]") == BitRange(2)); + CHECK(getBitRange(netlist, "foo[7]") == BitRange(3)); CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[3:0]") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[2:1]") == BitRange(1, 2)); + CHECK(getBitRange(netlist, "foo[5:4]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(0, 3)); + CHECK(getBitRange(netlist, "foo[6:5]") == BitRange(1, 2)); } //TEST_CASE("Packed 2D array element and range") { From c17958a734cc05b254ea4e76e20007d8fc10a7b4 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 18 Aug 2023 17:44:27 +0100 Subject: [PATCH 09/31] WIP scalar and array tests --- tools/netlist/include/SplitVariables.h | 21 ++++++++++++------- .../netlist/tests/VariableSelectorsTests.cpp | 5 +++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 0d3de4557..8c7db4a78 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -121,7 +121,7 @@ class AnalyseVariableReference { rightIndex -= arrayRange.right; auto* elementType = type.getArrayElementType(); auto newRange = BitRange(range.start + (rightIndex * elementType->getBitWidth()), - range.start + (leftIndex * elementType->getBitWidth()) - 1); + range.start + ((leftIndex + 1) * elementType->getBitWidth()) - 1); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -158,6 +158,15 @@ class AnalyseVariableReference { } // Simple vector if (type.isPredefinedInteger() || type.isScalar()) { + if (selectorsIt->get()->isRangeSelect() && + std::next(selectorsIt) != node.selectors.end() && + std::next(selectorsIt)->get()->isArraySelect()) { + // Multiple range selectors have only the effect of + // the last one. Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> + // x[2]. + selectorsIt++; + return getBitRangeImpl(type, range); + } if (selectorsIt->get()->isElementSelect()) { return handleScalarElementSelect(type, range); } @@ -170,12 +179,10 @@ class AnalyseVariableReference { } // Packed or unpacked array else if (type.isArray()) { - if (std::next(selectorsIt) != node.selectors.end() && - (selectorsIt->get()->isRangeSelect() && - std::next(selectorsIt)->get()->isArraySelect())) { - // Multiple range selectors have only the effect of - // the last one. Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> - // x[2]. + if (selectorsIt->get()->isRangeSelect() && + std::next(selectorsIt) != node.selectors.end() && + std::next(selectorsIt)->get()->isArraySelect()) { + // Multiple range selectors. selectorsIt++; return getBitRangeImpl(type, range); } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 7216135c4..1b377ef9a 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -46,6 +46,7 @@ endmodule CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); CHECK(getBitRange(netlist, "foo[3:1]") == BitRange(1, 3)); CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(4, 7)); + CHECK(getBitRange(netlist, "foo[3:1][2:1]") == BitRange(1, 2)); CHECK(getBitRange(netlist, "foo[7:4][6:5]") == BitRange(5, 6)); CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == BitRange(1)); CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(5)); @@ -64,6 +65,7 @@ module m; foo[1:0] = 0; foo[3:0] = 0; foo[2:1] = 0; + foo[3:1][2:1][1] = 0; end endmodule )"); @@ -79,6 +81,7 @@ endmodule CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); CHECK(getBitRange(netlist, "foo[3:0]") == BitRange(0, 3)); CHECK(getBitRange(netlist, "foo[2:1]") == BitRange(1, 2)); + CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == BitRange(1)); } TEST_CASE("Packed 1D array element and range non-zero indexed") { @@ -94,6 +97,7 @@ module m; foo[5:4] = 0; foo[7:4] = 0; foo[6:5] = 0; + foo[7:4][6:5][5] = 0; end endmodule )"); @@ -109,6 +113,7 @@ endmodule CHECK(getBitRange(netlist, "foo[5:4]") == BitRange(0, 1)); CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(0, 3)); CHECK(getBitRange(netlist, "foo[6:5]") == BitRange(1, 2)); + CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(1)); } //TEST_CASE("Packed 2D array element and range") { From a450abb0b702d0838254b6ffeeebc9398487d8a5 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 18 Aug 2023 21:06:56 +0100 Subject: [PATCH 10/31] WIP array access bit widths --- tools/netlist/TODO.md | 14 ++ .../netlist/tests/VariableSelectorsTests.cpp | 175 +++++++++++++----- 2 files changed, 141 insertions(+), 48 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index 6cc223fcb..678fe0535 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -1,6 +1,20 @@ To dos ====== +- Bug report: + +module m; + logic [7:4] [3:2] foo; + always_comb begin + foo[0] = 0; + end +endmodule + + Assertion 'range.left >= 0 && range.right >= 0' failed + in file /home/jamie/slang/source/ast/symbols/ValueSymbol.cpp, line 436 + function: static std::optional> slang::ast:: + ValueDriver::getBounds(const Expression &, EvalContext &, const Type &) + - Support for more procedural statements, the full list is: InvalidStatement diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 1b377ef9a..9a1edd480 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -8,21 +8,25 @@ #include "NetlistTest.h" #include "SplitVariables.h" +#include /// Helper method to extract a variable reference from a netlist and return the /// bit range associated with it. BitRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); + if (node == nullptr) { + throw std::runtime_error(fmt::format("Could not find node {}", variableSyntax)); + } return AnalyseVariableReference::create(*node).getBitRange(); } TEST_CASE("Scalar element and range") { - // Test element select on a scalar variable. auto tree = SyntaxTree::fromText(R"( module m; int foo; always_comb begin + foo = 0; foo[0] = 0; foo[1] = 0; foo[7:7] = 0; @@ -40,6 +44,7 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == BitRange(0, 31)); CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(7)); @@ -57,6 +62,7 @@ TEST_CASE("Packed 1D array element and range") { module m; logic [3:0] foo; always_comb begin + foo = 0; foo[0] = 0; foo[1] = 0; foo[2] = 0; @@ -73,6 +79,7 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == BitRange(0, 3)); CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); CHECK(getBitRange(netlist, "foo[2]") == BitRange(2)); @@ -89,6 +96,7 @@ TEST_CASE("Packed 1D array element and range non-zero indexed") { module m; logic [7:4] foo; always_comb begin + foo = 0; foo[4] = 0; foo[5] = 0; foo[6] = 0; @@ -105,6 +113,7 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == BitRange(0, 3)); CHECK(getBitRange(netlist, "foo[4]") == BitRange(0)); CHECK(getBitRange(netlist, "foo[5]") == BitRange(1)); CHECK(getBitRange(netlist, "foo[6]") == BitRange(2)); @@ -116,53 +125,123 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(1)); } -//TEST_CASE("Packed 2D array element and range") { -// // Test element select on a packed array variable. -// auto tree = SyntaxTree::fromText(R"( -//module m; -// logic [1:0] [1:0] foo; -// always_comb begin -// foo[0] = 0; -// foo[1] = 0; -// foo[0][0] = 0; -// foo[0][1] = 0; -// foo[1][0] = 0; -// foo[1][1] = 0; -// assign foo[3:2] = i_a; -// assign foo[1:0] = i_b; -// assign o_a = foo[7:4][6:5]; -// assign o_b = foo[3:0][2:1]; -// end -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -// CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); -// CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); -// CHECK(getBitRange(netlist, "foo[0][0]") == BitRange(0)); -// CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); -// CHECK(getBitRange(netlist, "foo[1][0]") == BitRange(2)); -// CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); -//} -// -//TEST_CASE("Unpacked 1D array element") { -// // Test element select on an unpacked array variable. -// auto tree = SyntaxTree::fromText(R"( -//module m; -// logic foo [1:0]; -// always_comb begin -// foo[0] = 0; -// foo[1] = 0; -// end -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -//} +TEST_CASE("Packed 2D array element and range") { + auto tree = SyntaxTree::fromText(R"( +module m; + logic [3:0] [1:0] foo; + always_comb begin + foo = 0; + foo[0] = 0; + foo[1] = 0; + foo[2] = 0; + foo[3] = 0; + foo[0][1] = 0; + foo[1][1] = 0; + foo[2][1] = 0; + foo[3][1] = 0; + foo[1:0] = 0; + foo[3:2] = 0; + foo[3:0][2:1] = 0; + foo[3:0][2:1][1] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == BitRange(0, 7)); + CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); + CHECK(getBitRange(netlist, "foo[2]") == BitRange(4, 5)); + CHECK(getBitRange(netlist, "foo[3]") == BitRange(6, 7)); + CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); + CHECK(getBitRange(netlist, "foo[2][1]") == BitRange(5)); + CHECK(getBitRange(netlist, "foo[3][1]") == BitRange(7)); + CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 3)); + CHECK(getBitRange(netlist, "foo[3:2]") == BitRange(4, 7)); + CHECK(getBitRange(netlist, "foo[3:0][2:1]") == BitRange(2, 5)); + CHECK(getBitRange(netlist, "foo[3:0][2:1][1]") == BitRange(2, 3)); +} + +TEST_CASE("Packed 2D array element and range, non-zero indexing") { + auto tree = SyntaxTree::fromText(R"( +module m; + logic [7:4] [3:2] foo; + always_comb begin + foo = 0; + foo[4] = 0; + foo[4][3] = 0; + foo[5:4] = 0; + foo[7:4][6:5] = 0; + foo[7:5][6:5][5] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == BitRange(0, 7)); + CHECK(getBitRange(netlist, "foo[4]") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[4][3]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo[5:4]") == BitRange(0, 3)); + CHECK(getBitRange(netlist, "foo[7:4][6:5]") == BitRange(2, 5)); + CHECK(getBitRange(netlist, "foo[7:5][6:5][5]") == BitRange(2, 3)); +} + +TEST_CASE("Unpacked 1D array element") { + auto tree = SyntaxTree::fromText(R"( +module m; + logic foo [1:0]; + logic bar [1:0]; + always_comb begin + foo = bar; + foo[0] = 0; + foo[1] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + // Can't get bitwidth of an unpacked type. + //CHECK(getBitRange(netlist, "foo") == BitRange(0, 1)); + CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); + CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); +} + +TEST_CASE("Unpacked 2D array element") { + auto tree = SyntaxTree::fromText(R"( +module m; + logic foo [3:0] [1:0]; + logic bar [1:0]; + always_comb begin + foo[0] = bar; + foo[1] = bar; + foo[2] = bar; + foo[3] = bar; + foo[0][1] = 0; + foo[1][1] = 0; + foo[2][1] = 0; + foo[3][1] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + // Can't get bitwidth of an unpacked type. + //CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); + //CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); + //CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); + //CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); + //CHECK(getBitRange(netlist, "foo[2][1]") == BitRange(5)); + //CHECK(getBitRange(netlist, "foo[3][1]") == BitRange(7)); +} //===---------------------------------------------------------------------===// // Tests for variable splitting From 91fb3b41736c3321c0f3f09609c2c6718cabc2ec Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 20 Aug 2023 21:31:41 +0100 Subject: [PATCH 11/31] Switch to using ConstantRange --- tools/netlist/TODO.md | 12 +- tools/netlist/include/BitRange.h | 45 ------- tools/netlist/include/SplitVariables.h | 97 +++++++------- .../netlist/tests/VariableSelectorsTests.cpp | 120 +++++++++--------- 4 files changed, 115 insertions(+), 159 deletions(-) delete mode 100644 tools/netlist/include/BitRange.h diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index 678fe0535..c9b20c348 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -3,12 +3,12 @@ To dos - Bug report: -module m; - logic [7:4] [3:2] foo; - always_comb begin - foo[0] = 0; - end -endmodule + module m; + logic [7:4] [3:2] foo; + always_comb begin + foo[0] = 0; + end + endmodule Assertion 'range.left >= 0 && range.right >= 0' failed in file /home/jamie/slang/source/ast/symbols/ValueSymbol.cpp, line 436 diff --git a/tools/netlist/include/BitRange.h b/tools/netlist/include/BitRange.h deleted file mode 100644 index 568440f43..000000000 --- a/tools/netlist/include/BitRange.h +++ /dev/null @@ -1,45 +0,0 @@ -//------------------------------------------------------------------------------ -//! @file BitRange.h -//! @brieif A class to represent a bit range. -// -// SPDX-FileCopyrightText: Michael Popoloski -// SPDX-License-Identifier: MIT -//------------------------------------------------------------------------------ -#pragma once - -#include -#include -#include "fmt/format.h" - -#include "slang/numeric/SVInt.h" - - -struct BitRange { - slang::bitwidth_t start; - slang::bitwidth_t end; - BitRange(size_t index) : start(index), end(index) {} - BitRange(size_t start, size_t end) : start(start), end(end) { - SLANG_ASSERT(start <= end); - } - /// Given another range, return true if it overlaps with this range. - inline bool overlap(BitRange other) const { - return start <= other.end && other.start <= end; - } - /// Given another range, return true if it is contained within this range. - inline bool contains(BitRange other) const { - return other.start >= start && other.end <= end; - } - /// Equality. - friend bool operator==(BitRange const & A, BitRange const & B) noexcept { - return A.start == B.start && A.end == B.end; - } - // Output to stream. - friend std::ostream& operator<<(std::ostream& os, const BitRange& range) { - os << fmt::format("BitRange {}", range.toString()); - return os; - } - size_t size() const { return end - start; } - std::string toString() const { return fmt::format("[{}:{}]", end, start); } -}; - - diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 8c7db4a78..4f6b5a48c 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -16,9 +16,10 @@ #include "slang/ast/Symbol.h" #include "slang/ast/types/AllTypes.h" #include "slang/ast/types/Type.h" +#include "slang/numeric/ConstantValue.h" #include "slang/numeric/SVInt.h" #include "slang/util/Util.h" -#include "BitRange.h" + namespace netlist { @@ -36,13 +37,13 @@ class AnalyseVariableReference { node(node), selectorsIt(node.selectors.begin()) {} /// Given a packed struct type, return the bit position of the named field. - BitRange getFieldRange(const slang::ast::PackedStructType &packedStruct, - const std::string_view fieldName) const { + ConstantRange getFieldRange(const slang::ast::PackedStructType &packedStruct, + const std::string_view fieldName) const { size_t offset = 0; for (auto &member : packedStruct.members()) { - auto fieldWidth = member.getDeclaredType()->getType().getBitWidth(); + int32_t fieldWidth = member.getDeclaredType()->getType().getBitWidth(); if (member.name == fieldName) { - return BitRange(offset, offset+fieldWidth); + return {(int32_t) offset, (int32_t) offset + fieldWidth}; }; offset += fieldWidth;; } @@ -50,7 +51,7 @@ class AnalyseVariableReference { } /// Given an array type, return the range from which the array is indexed. - const slang::ConstantRange& getArrayRange(const slang::ast::Type &type) { + ConstantRange getArrayRange(const slang::ast::Type &type) { if (type.kind == slang::ast::SymbolKind::PackedArrayType) { auto& arrayType = type.as(); return arrayType.range; @@ -64,23 +65,23 @@ class AnalyseVariableReference { } } - BitRange handleScalarElementSelect(const slang::ast::Type &type, BitRange range) { + ConstantRange handleScalarElementSelect(const slang::ast::Type &type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); - SLANG_ASSERT(elementSelector.getIndexInt() >= range.start); - SLANG_ASSERT(elementSelector.getIndexInt() <= range.end); - size_t index = range.start + elementSelector.getIndexInt(); - return BitRange(index); + SLANG_ASSERT(elementSelector.getIndexInt() >= range.lower()); + SLANG_ASSERT(elementSelector.getIndexInt() <= range.upper()); + int32_t index = range.lower() + elementSelector.getIndexInt(); + return {index, index}; } - BitRange handleScalarRangeSelect(const slang::ast::Type &type, BitRange range) { + ConstantRange handleScalarRangeSelect(const slang::ast::Type &type, ConstantRange range) { const auto& rangeSelector = selectorsIt->get()->as(); - slang::bitwidth_t leftIndex = rangeSelector.getLeftIndexInt(); - slang::bitwidth_t rightIndex = rangeSelector.getRightIndexInt(); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); + int32_t rightIndex = rangeSelector.getRightIndexInt(); SLANG_ASSERT(rightIndex <= leftIndex); - SLANG_ASSERT(rightIndex >= range.start); - SLANG_ASSERT(leftIndex <= range.end); - auto newRange = BitRange(range.start + rightIndex, - range.start + leftIndex); + SLANG_ASSERT(rightIndex >= range.lower()); + SLANG_ASSERT(leftIndex <= range.upper()); + ConstantRange newRange = {range.lower() + rightIndex, + range.lower() + leftIndex}; if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -89,17 +90,17 @@ class AnalyseVariableReference { } } - BitRange handleArrayElementSelect(const slang::ast::Type &type, BitRange range) { + ConstantRange handleArrayElementSelect(const slang::ast::Type &type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); - size_t index = elementSelector.getIndexInt(); - auto& arrayRange = getArrayRange(type); - SLANG_ASSERT(index >= arrayRange.right); - SLANG_ASSERT(index <= arrayRange.left); + int32_t index = elementSelector.getIndexInt(); + auto arrayRange = getArrayRange(type); + SLANG_ASSERT(index >= arrayRange.lower()); + SLANG_ASSERT(index <= arrayRange.upper()); // Adjust for non-zero array indexing. - index -= arrayRange.right; + index -= arrayRange.lower(); auto* elementType = type.getArrayElementType(); - auto newRange = BitRange(range.start + (index * elementType->getBitWidth()), - range.start + ((index + 1) * elementType->getBitWidth()) - 1); + ConstantRange newRange = {range.lower() + (index * (int32_t) elementType->getBitWidth()), + range.lower() + ((index + 1) * (int32_t) elementType->getBitWidth()) - 1}; if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); @@ -108,20 +109,20 @@ class AnalyseVariableReference { } } - BitRange handleArrayRangeSelect(const slang::ast::Type &type, BitRange range) { + ConstantRange handleArrayRangeSelect(const slang::ast::Type &type, ConstantRange range) { const auto& rangeSelector = selectorsIt->get()->as(); - size_t leftIndex = rangeSelector.getLeftIndexInt(); - size_t rightIndex = rangeSelector.getRightIndexInt(); - auto& arrayRange = getArrayRange(type); - SLANG_ASSERT(rightIndex >= arrayRange.right); - SLANG_ASSERT(leftIndex <= arrayRange.left); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); + int32_t rightIndex = rangeSelector.getRightIndexInt(); + auto arrayRange = getArrayRange(type); + SLANG_ASSERT(rightIndex >= arrayRange.lower()); + SLANG_ASSERT(leftIndex <= arrayRange.upper()); SLANG_ASSERT(rightIndex <= leftIndex); // Adjust for non-zero array indexing. - leftIndex -= arrayRange.right; - rightIndex -= arrayRange.right; + leftIndex -= arrayRange.lower(); + rightIndex -= arrayRange.lower(); auto* elementType = type.getArrayElementType(); - auto newRange = BitRange(range.start + (rightIndex * elementType->getBitWidth()), - range.start + ((leftIndex + 1) * elementType->getBitWidth()) - 1); + ConstantRange newRange = {range.lower() + (rightIndex * (int32_t) elementType->getBitWidth()), + range.lower() + ((leftIndex + 1) * (int32_t) elementType->getBitWidth()) - 1}; if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -130,7 +131,7 @@ class AnalyseVariableReference { } } - BitRange handleStructMemberAccess(const slang::ast::Type &type, BitRange range) { + ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { //const auto& packedStruct = type.getCanonicalType().as(); //SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); //const auto& memberAccessSelector = selectorsIt->get()->as(); @@ -138,23 +139,23 @@ class AnalyseVariableReference { //SLANG_ASSERT(range.contains(fieldRange)); //auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; //return getBitRange(fieldType, fieldRange.start, fieldRange.end); - return BitRange(0); + return {0, 0}; } - BitRange handleUnionMemberAccess(const slang::ast::Type &type, BitRange range) { - return BitRange(0); + ConstantRange handleUnionMemberAccess(const slang::ast::Type &type, ConstantRange range) { + return {0, 0}; } - BitRange handleEnumMemberAccess(const slang::ast::Type &type, BitRange range) { - return BitRange(0); + ConstantRange handleEnumMemberAccess(const slang::ast::Type &type, ConstantRange range) { + return {0, 0}; } /// Given a variable reference with zero or more selectors, determine the /// bit range that is accessed. - BitRange getBitRangeImpl(const slang::ast::Type &type, BitRange range) { + ConstantRange getBitRangeImpl(const slang::ast::Type &type, ConstantRange range) { // No selectors if (node.selectors.empty()) { - return BitRange(0, node.symbol.getDeclaredType()->getType().getBitWidth()-1); + return {0, (int32_t) node.symbol.getDeclaredType()->getType().getBitWidth() - 1}; } // Simple vector if (type.isPredefinedInteger() || type.isScalar()) { @@ -226,9 +227,9 @@ class AnalyseVariableReference { } /// Return a range indicating the bits of the variable that are accessed. - BitRange getBitRange() { - auto& variableType = node.symbol.getDeclaredType()->getType(); - return getBitRangeImpl(variableType, BitRange(0, variableType.getBitWidth()-1)); + ConstantRange getBitRange() { + auto& variableType = node.symbol.getDeclaredType()->getType(); + return getBitRangeImpl(variableType, {0, (int32_t) variableType.getBitWidth() - 1}); } }; @@ -245,7 +246,7 @@ class SplitVariables { /// selection made by the source node. bool isIntersectingSelection(NetlistVariableReference& sourceNode, NetlistVariableReference& targetNode) const { - return AnalyseVariableReference(sourceNode).getBitRange().overlap( + return AnalyseVariableReference(sourceNode).getBitRange().overlaps( AnalyseVariableReference(targetNode).getBitRange()); } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 9a1edd480..a0c975d43 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -13,7 +13,7 @@ /// Helper method to extract a variable reference from a netlist and return the /// bit range associated with it. -BitRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { +ConstantRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); if (node == nullptr) { throw std::runtime_error(fmt::format("Could not find node {}", variableSyntax)); @@ -44,17 +44,17 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == BitRange(0, 31)); - CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); - CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(7)); - CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[3:1]") == BitRange(1, 3)); - CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(4, 7)); - CHECK(getBitRange(netlist, "foo[3:1][2:1]") == BitRange(1, 2)); - CHECK(getBitRange(netlist, "foo[7:4][6:5]") == BitRange(5, 6)); - CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(5)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[7:7]") == ConstantRange(7, 7)); + CHECK(getBitRange(netlist, "foo[1:0]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[3:1]") == ConstantRange(1, 3)); + CHECK(getBitRange(netlist, "foo[7:4]") == ConstantRange(4, 7)); + CHECK(getBitRange(netlist, "foo[3:1][2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(5, 6)); + CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(5, 5)); } TEST_CASE("Packed 1D array element and range") { @@ -79,16 +79,16 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); - CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[2]") == BitRange(2)); - CHECK(getBitRange(netlist, "foo[3]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[3:3]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[3:0]") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[2:1]") == BitRange(1, 2)); - CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[2]") == ConstantRange(2, 2)); + CHECK(getBitRange(netlist, "foo[3]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[3:3]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[1:0]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[3:0]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); } TEST_CASE("Packed 1D array element and range non-zero indexed") { @@ -113,16 +113,16 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[4]") == BitRange(0)); - CHECK(getBitRange(netlist, "foo[5]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[6]") == BitRange(2)); - CHECK(getBitRange(netlist, "foo[7]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[7:7]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[5:4]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[7:4]") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[6:5]") == BitRange(1, 2)); - CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == BitRange(1)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[4]") == ConstantRange(0, 0)); + CHECK(getBitRange(netlist, "foo[5]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[6]") == ConstantRange(2, 2)); + CHECK(getBitRange(netlist, "foo[7]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[7:7]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[5:4]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[7:4]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[6:5]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(1, 1)); } TEST_CASE("Packed 2D array element and range") { @@ -150,19 +150,19 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == BitRange(0, 7)); - CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); - CHECK(getBitRange(netlist, "foo[2]") == BitRange(4, 5)); - CHECK(getBitRange(netlist, "foo[3]") == BitRange(6, 7)); - CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); - CHECK(getBitRange(netlist, "foo[2][1]") == BitRange(5)); - CHECK(getBitRange(netlist, "foo[3][1]") == BitRange(7)); - CHECK(getBitRange(netlist, "foo[1:0]") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[3:2]") == BitRange(4, 7)); - CHECK(getBitRange(netlist, "foo[3:0][2:1]") == BitRange(2, 5)); - CHECK(getBitRange(netlist, "foo[3:0][2:1][1]") == BitRange(2, 3)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[2]") == ConstantRange(4, 5)); + CHECK(getBitRange(netlist, "foo[3]") == ConstantRange(6, 7)); + CHECK(getBitRange(netlist, "foo[0][1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[1][1]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[2][1]") == ConstantRange(5, 5)); + CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(7, 7)); + CHECK(getBitRange(netlist, "foo[1:0]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[3:2]") == ConstantRange(4, 7)); + CHECK(getBitRange(netlist, "foo[3:0][2:1]") == ConstantRange(2, 5)); + CHECK(getBitRange(netlist, "foo[3:0][2:1][1]") == ConstantRange(2, 3)); } TEST_CASE("Packed 2D array element and range, non-zero indexing") { @@ -183,12 +183,12 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == BitRange(0, 7)); - CHECK(getBitRange(netlist, "foo[4]") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[4][3]") == BitRange(1)); - CHECK(getBitRange(netlist, "foo[5:4]") == BitRange(0, 3)); - CHECK(getBitRange(netlist, "foo[7:4][6:5]") == BitRange(2, 5)); - CHECK(getBitRange(netlist, "foo[7:5][6:5][5]") == BitRange(2, 3)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[4]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[4][3]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[5:4]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(2, 5)); + CHECK(getBitRange(netlist, "foo[7:5][6:5][5]") == ConstantRange(2, 3)); } TEST_CASE("Unpacked 1D array element") { @@ -208,9 +208,9 @@ endmodule NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); // Can't get bitwidth of an unpacked type. - //CHECK(getBitRange(netlist, "foo") == BitRange(0, 1)); - CHECK(getBitRange(netlist, "foo[0]") == BitRange(0)); - CHECK(getBitRange(netlist, "foo[1]") == BitRange(1)); + //CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 1)); + //CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); + //CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); } TEST_CASE("Unpacked 2D array element") { @@ -235,12 +235,12 @@ endmodule NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); // Can't get bitwidth of an unpacked type. - //CHECK(getBitRange(netlist, "foo[0]") == BitRange(0, 1)); - //CHECK(getBitRange(netlist, "foo[1]") == BitRange(2, 3)); - //CHECK(getBitRange(netlist, "foo[0][1]") == BitRange(1)); - //CHECK(getBitRange(netlist, "foo[1][1]") == BitRange(3)); - //CHECK(getBitRange(netlist, "foo[2][1]") == BitRange(5)); - //CHECK(getBitRange(netlist, "foo[3][1]") == BitRange(7)); + //CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 1)); + //CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(2, 3)); + //CHECK(getBitRange(netlist, "foo[0][1]") == ConstantRange(1)); + //CHECK(getBitRange(netlist, "foo[1][1]") == ConstantRange(3)); + //CHECK(getBitRange(netlist, "foo[2][1]") == ConstantRange(5)); + //CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(7)); } //===---------------------------------------------------------------------===// From 3e4de3ceb8ff1aac29c3d95c8196e29a7e51fbd6 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 20 Aug 2023 21:36:04 +0100 Subject: [PATCH 12/31] Use int32_t for select indices --- tools/netlist/include/Netlist.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 771becb1c..20d9d2618 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -76,9 +76,9 @@ struct VariableElementSelect : public VariableSelectorBase { return otherKind == VariableSelectorKind::ElementSelect; } - size_t getIndexInt() const { - auto intValue = index.integer().as(); - SLANG_ASSERT(intValue && "could not convert index to size_t"); + int32_t getIndexInt() const { + auto intValue = index.integer().as(); + SLANG_ASSERT(intValue && "could not convert index to int32_t"); return *intValue; } @@ -98,15 +98,15 @@ struct VariableRangeSelect : public VariableSelectorBase { return otherKind == VariableSelectorKind::RangeSelect; } - size_t getLeftIndexInt() const { - auto intValue = leftIndex.integer().as(); - SLANG_ASSERT(intValue && "could not convert left index to size_t"); + int32_t getLeftIndexInt() const { + auto intValue = leftIndex.integer().as(); + SLANG_ASSERT(intValue && "could not convert left index to int32_t"); return *intValue; } - size_t getRightIndexInt() const { - auto intValue = rightIndex.integer().as(); - SLANG_ASSERT(intValue && "could not convert right index to size_t"); + int32_t getRightIndexInt() const { + auto intValue = rightIndex.integer().as(); + SLANG_ASSERT(intValue && "could not convert right index to int32_t"); return *intValue; } From 51b28fefdc5dfc83b57c3e6964b581fc9a2fdbd2 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Mon, 28 Aug 2023 20:12:10 +0100 Subject: [PATCH 13/31] Fix unpacked array cases --- tools/netlist/include/SplitVariables.h | 36 +++++++++++++++---- .../netlist/tests/VariableSelectorsTests.cpp | 20 +++++------ 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 4f6b5a48c..2997db62c 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -36,6 +36,30 @@ class AnalyseVariableReference { AnalyseVariableReference(NetlistVariableReference &node) : node(node), selectorsIt(node.selectors.begin()) {} + std::pair getTypeBitWidthImpl(slang::ast::Type const& type) { + size_t fixedSize = type.getBitWidth(); + if (fixedSize > 0) { + return {1, fixedSize}; + } + + size_t multiplier = 0; + const auto& ct = type.getCanonicalType(); + if (ct.kind == slang::ast::SymbolKind::FixedSizeUnpackedArrayType) { + auto [multiplierElem, fixedSizeElem] = getTypeBitWidthImpl(*type.getArrayElementType()); + auto rw = ct.as().range.width(); + return {multiplierElem * rw, fixedSizeElem}; + } + + SLANG_ASSERT(0 && "unsupported type for getTypeBitWidth"); + } + + /// Return the bit width of a slang type, treating unpacked arrays as + /// as if they were packed. + int32_t getTypeBitWidth(slang::ast::Type const &type) { + auto [multiplierElem, fixedSizeElem] = getTypeBitWidthImpl(type); + return (int32_t) (multiplierElem * fixedSizeElem); + } + /// Given a packed struct type, return the bit position of the named field. ConstantRange getFieldRange(const slang::ast::PackedStructType &packedStruct, const std::string_view fieldName) const { @@ -99,8 +123,8 @@ class AnalyseVariableReference { // Adjust for non-zero array indexing. index -= arrayRange.lower(); auto* elementType = type.getArrayElementType(); - ConstantRange newRange = {range.lower() + (index * (int32_t) elementType->getBitWidth()), - range.lower() + ((index + 1) * (int32_t) elementType->getBitWidth()) - 1}; + ConstantRange newRange = {range.lower() + (index * getTypeBitWidth(*elementType)), + range.lower() + ((index + 1) * getTypeBitWidth(*elementType)) - 1}; if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); @@ -121,8 +145,8 @@ class AnalyseVariableReference { leftIndex -= arrayRange.lower(); rightIndex -= arrayRange.lower(); auto* elementType = type.getArrayElementType(); - ConstantRange newRange = {range.lower() + (rightIndex * (int32_t) elementType->getBitWidth()), - range.lower() + ((leftIndex + 1) * (int32_t) elementType->getBitWidth()) - 1}; + ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), + range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -155,7 +179,7 @@ class AnalyseVariableReference { ConstantRange getBitRangeImpl(const slang::ast::Type &type, ConstantRange range) { // No selectors if (node.selectors.empty()) { - return {0, (int32_t) node.symbol.getDeclaredType()->getType().getBitWidth() - 1}; + return {0, getTypeBitWidth(node.symbol.getDeclaredType()->getType()) - 1}; } // Simple vector if (type.isPredefinedInteger() || type.isScalar()) { @@ -229,7 +253,7 @@ class AnalyseVariableReference { /// Return a range indicating the bits of the variable that are accessed. ConstantRange getBitRange() { auto& variableType = node.symbol.getDeclaredType()->getType(); - return getBitRangeImpl(variableType, {0, (int32_t) variableType.getBitWidth() - 1}); + return getBitRangeImpl(variableType, {0, getTypeBitWidth(variableType) - 1}); } }; diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index a0c975d43..29c20c057 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -207,10 +207,9 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - // Can't get bitwidth of an unpacked type. - //CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 1)); - //CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); - //CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); } TEST_CASE("Unpacked 2D array element") { @@ -234,13 +233,12 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - // Can't get bitwidth of an unpacked type. - //CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 1)); - //CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(2, 3)); - //CHECK(getBitRange(netlist, "foo[0][1]") == ConstantRange(1)); - //CHECK(getBitRange(netlist, "foo[1][1]") == ConstantRange(3)); - //CHECK(getBitRange(netlist, "foo[2][1]") == ConstantRange(5)); - //CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(7)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[0][1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[1][1]") == ConstantRange(3, 3)); + CHECK(getBitRange(netlist, "foo[2][1]") == ConstantRange(5, 5)); + CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(7, 7)); } //===---------------------------------------------------------------------===// From ea6a193c2415b1bc7cb400ab1d7ae58e4f7d128b Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Tue, 29 Aug 2023 21:05:59 +0100 Subject: [PATCH 14/31] Add variants for range selectors Refactor expression Start adding logic for non-constant selects --- tools/netlist/include/Netlist.h | 20 +++++- tools/netlist/include/SplitVariables.h | 71 +++++++++++++++---- .../netlist/tests/VariableSelectorsTests.cpp | 5 +- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 20d9d2618..05b548596 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -76,6 +76,10 @@ struct VariableElementSelect : public VariableSelectorBase { return otherKind == VariableSelectorKind::ElementSelect; } + bool indexIsConstant() const { + return !index.bad(); + } + int32_t getIndexInt() const { auto intValue = index.integer().as(); SLANG_ASSERT(intValue && "could not convert index to int32_t"); @@ -90,14 +94,24 @@ struct VariableRangeSelect : public VariableSelectorBase { ConstantValue leftIndex, rightIndex; ast::RangeSelectionKind selectionKind; - VariableRangeSelect(ConstantValue leftIndex, ConstantValue rightIndex, ast::RangeSelectionKind selectionKind) : - VariableSelectorBase(VariableSelectorKind::RangeSelect), leftIndex(std::move(leftIndex)), - rightIndex(std::move(rightIndex)), selectionKind(selectionKind) {} + VariableRangeSelect(ConstantValue leftIndex, ConstantValue rightIndex, + ast::RangeSelectionKind selectionKind) : + VariableSelectorBase(VariableSelectorKind::RangeSelect), + leftIndex(std::move(leftIndex)), rightIndex(std::move(rightIndex)), + selectionKind(selectionKind) {} static bool isKind(VariableSelectorKind otherKind) { return otherKind == VariableSelectorKind::RangeSelect; } + bool leftIndexIsConstant() const { + return !leftIndex.bad(); + } + + bool rightIndexIsConstant() const { + return !rightIndex.bad(); + } + int32_t getLeftIndexInt() const { auto intValue = leftIndex.integer().as(); SLANG_ASSERT(intValue && "could not convert left index to int32_t"); diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 2997db62c..64002baa4 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -89,8 +89,13 @@ class AnalyseVariableReference { } } - ConstantRange handleScalarElementSelect(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleScalarElementSelect(const slang::ast::Type& type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); + if (!elementSelector.indexIsConstant()) { + // If the selector is not a constant, then return the whole scalar as + // the range. + return {range.lower(), (int32_t) type.getBitWidth()}; + } SLANG_ASSERT(elementSelector.getIndexInt() >= range.lower()); SLANG_ASSERT(elementSelector.getIndexInt() <= range.upper()); int32_t index = range.lower() + elementSelector.getIndexInt(); @@ -99,8 +104,10 @@ class AnalyseVariableReference { ConstantRange handleScalarRangeSelect(const slang::ast::Type &type, ConstantRange range) { const auto& rangeSelector = selectorsIt->get()->as(); - int32_t leftIndex = rangeSelector.getLeftIndexInt(); int32_t rightIndex = rangeSelector.getRightIndexInt(); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); + //if (!rangeSelector.leftIndexIsConstant()) { + //} SLANG_ASSERT(rightIndex <= leftIndex); SLANG_ASSERT(rightIndex >= range.lower()); SLANG_ASSERT(leftIndex <= range.upper()); @@ -114,8 +121,19 @@ class AnalyseVariableReference { } } + ConstantRange handleScalarRangeSelectUp(const slang::ast::Type &type, ConstantRange range) { + } + + ConstantRange handleScalarRangeSelectDown(const slang::ast::Type &type, ConstantRange range) { + } + ConstantRange handleArrayElementSelect(const slang::ast::Type &type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); + if (!elementSelector.indexIsConstant()) { + // If the selector is not a constant, then return the whole scalar as + // the range. + return {range.lower(), (int32_t) type.getBitWidth()}; + } int32_t index = elementSelector.getIndexInt(); auto arrayRange = getArrayRange(type); SLANG_ASSERT(index >= arrayRange.lower()); @@ -155,6 +173,12 @@ class AnalyseVariableReference { } } + ConstantRange handleArrayRangeSelectUp(const slang::ast::Type &type, ConstantRange range) { + } + + ConstantRange handleArrayRangeSelectDown(const slang::ast::Type &type, ConstantRange range) { + } + ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { //const auto& packedStruct = type.getCanonicalType().as(); //SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); @@ -174,6 +198,15 @@ class AnalyseVariableReference { return {0, 0}; } + + // Multiple range selectors have only the effect of the last one. + // Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> x[2]. + inline bool ignoreSelector() { + return selectorsIt->get()->isRangeSelect() && + std::next(selectorsIt) != node.selectors.end() && + std::next(selectorsIt)->get()->isArraySelect(); + } + /// Given a variable reference with zero or more selectors, determine the /// bit range that is accessed. ConstantRange getBitRangeImpl(const slang::ast::Type &type, ConstantRange range) { @@ -183,12 +216,7 @@ class AnalyseVariableReference { } // Simple vector if (type.isPredefinedInteger() || type.isScalar()) { - if (selectorsIt->get()->isRangeSelect() && - std::next(selectorsIt) != node.selectors.end() && - std::next(selectorsIt)->get()->isArraySelect()) { - // Multiple range selectors have only the effect of - // the last one. Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> - // x[2]. + if (ignoreSelector()) { selectorsIt++; return getBitRangeImpl(type, range); } @@ -196,7 +224,16 @@ class AnalyseVariableReference { return handleScalarElementSelect(type, range); } else if (selectorsIt->get()->isRangeSelect()) { - return handleScalarRangeSelect(type, range); + switch (selectorsIt->get()->as().selectionKind) { + case ast::RangeSelectionKind::Simple: + return handleScalarRangeSelect(type, range); + case ast::RangeSelectionKind::IndexedUp: + return handleScalarRangeSelectUp(type, range); + case ast::RangeSelectionKind::IndexedDown: + return handleScalarRangeSelectDown(type, range); + default: + SLANG_UNREACHABLE; + } } else { SLANG_ASSERT(0 && "unsupported scalar selector"); @@ -204,10 +241,7 @@ class AnalyseVariableReference { } // Packed or unpacked array else if (type.isArray()) { - if (selectorsIt->get()->isRangeSelect() && - std::next(selectorsIt) != node.selectors.end() && - std::next(selectorsIt)->get()->isArraySelect()) { - // Multiple range selectors. + if (ignoreSelector()) { selectorsIt++; return getBitRangeImpl(type, range); } @@ -215,7 +249,16 @@ class AnalyseVariableReference { return handleArrayElementSelect(type, range); } else if (selectorsIt->get()->isRangeSelect()) { - return handleArrayRangeSelect(type, range); + switch (selectorsIt->get()->as().selectionKind) { + case ast::RangeSelectionKind::Simple: + return handleArrayRangeSelect(type, range); + case ast::RangeSelectionKind::IndexedUp: + return handleArrayRangeSelectUp(type, range); + case ast::RangeSelectionKind::IndexedDown: + return handleArrayRangeSelectDown(type, range); + default: + SLANG_UNREACHABLE; + } } else { SLANG_ASSERT(0 && "unsupported array selector"); diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 29c20c057..d068f7be4 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -23,7 +23,7 @@ ConstantRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { TEST_CASE("Scalar element and range") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); int foo; always_comb begin foo = 0; @@ -37,6 +37,7 @@ module m; foo[7:4][6:5] = 0; foo[3:1][2:1][1] = 0; foo[7:4][6:5][5] = 0; + foo[a] = 0; end endmodule )"); @@ -55,6 +56,8 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(5, 6)); CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(5, 5)); + // Can't lookup foo[a] in the netlist. + //CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 31)); } TEST_CASE("Packed 1D array element and range") { From 2c19b0e915e96d7691ca696149a59523ea9a1b2a Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 10 Sep 2023 22:33:35 +0100 Subject: [PATCH 15/31] WIP --- tools/netlist/TODO.md | 5 ++ tools/netlist/include/Netlist.h | 59 +++++++++++++++---- tools/netlist/include/NetlistVisitor.h | 13 ++-- tools/netlist/include/SplitVariables.h | 4 +- .../netlist/tests/VariableSelectorsTests.cpp | 11 +++- 5 files changed, 69 insertions(+), 23 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index c9b20c348..da2bb96b9 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -1,6 +1,11 @@ To dos ====== +- Bug report: + + In ast::ElementSelectExpr.syntax does not correspond to selector source + range and includes entire variable reference + selectors. + - Bug report: module m; diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 05b548596..c8b4b4436 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -67,10 +67,12 @@ struct VariableSelectorBase { /// A variable selector representing an element selector. struct VariableElementSelect : public VariableSelectorBase { + const ast::ElementSelectExpression &expr; ConstantValue index; - VariableElementSelect(ConstantValue index) : - VariableSelectorBase(VariableSelectorKind::ElementSelect), index(std::move(index)) {} + VariableElementSelect(ast::ElementSelectExpression const& expr, ConstantValue index) : + VariableSelectorBase(VariableSelectorKind::ElementSelect), expr(expr), + index(std::move(index)) {} static bool isKind(VariableSelectorKind otherKind) { return otherKind == VariableSelectorKind::ElementSelect; @@ -86,19 +88,24 @@ struct VariableElementSelect : public VariableSelectorBase { return *intValue; } - std::string toString() const override { return fmt::format("[{}]", index.toString()); } + std::string toString() const override { + if (indexIsConstant()) { + return fmt::format("[{}]", index.toString()); + } else { + return fmt::format("[{}]", expr.syntax->toString()); + } + } }; /// A variable selector representing a range selector. struct VariableRangeSelect : public VariableSelectorBase { + const ast::RangeSelectExpression &expr; ConstantValue leftIndex, rightIndex; - ast::RangeSelectionKind selectionKind; - VariableRangeSelect(ConstantValue leftIndex, ConstantValue rightIndex, - ast::RangeSelectionKind selectionKind) : + VariableRangeSelect(ast::RangeSelectExpression const& expr, ConstantValue leftIndex, + ConstantValue rightIndex) : VariableSelectorBase(VariableSelectorKind::RangeSelect), - leftIndex(std::move(leftIndex)), rightIndex(std::move(rightIndex)), - selectionKind(selectionKind) {} + expr(expr), leftIndex(std::move(leftIndex)), rightIndex(std::move(rightIndex)) {} static bool isKind(VariableSelectorKind otherKind) { return otherKind == VariableSelectorKind::RangeSelect; @@ -125,7 +132,28 @@ struct VariableRangeSelect : public VariableSelectorBase { } std::string toString() const override { - return fmt::format("[{}:{}]", leftIndex.toString(), rightIndex.toString()); + std::string left; + if (leftIndexIsConstant()) { + left = leftIndex.toString(); + } else { + left = expr.left().syntax->toString(); + } + std::string right; + if (rightIndexIsConstant()) { + right = rightIndex.toString(); + } else { + right = expr.right().syntax->toString(); + } + switch (expr.getSelectionKind()) { + case ast::RangeSelectionKind::Simple: + return fmt::format("[{}:{}]", left, right); + case ast::RangeSelectionKind::IndexedUp: + return fmt::format("[{}+:{}]", left, right); + case ast::RangeSelectionKind::IndexedDown: + return fmt::format("[{}-:{}]", left, right); + default: + SLANG_UNREACHABLE; + } } }; @@ -243,12 +271,17 @@ class NetlistVariableReference : public NetlistNode { NetlistNode(NodeKind::VariableReference, symbol), expression(expr), leftOperand(leftOperand) {} - void addElementSelect(const ConstantValue& index) { - selectors.emplace_back(std::make_unique(index)); + void addElementSelect(ast::ElementSelectExpression const &expr, const ConstantValue& index) { + selectors.emplace_back(std::make_unique(expr, index)); + //std::cout << "Add elem select "<< expr.syntax->toString() << "\n"; } - void addRangeSelect(const ConstantValue& leftIndex, const ConstantValue& rightIndex, ast::RangeSelectionKind selectionKind) { - selectors.emplace_back(std::make_unique(leftIndex, rightIndex, selectionKind)); + + void addRangeSelect(ast::RangeSelectExpression const& expr, + const ConstantValue& leftIndex, const ConstantValue& rightIndex) { + selectors.emplace_back(std::make_unique(expr, leftIndex, rightIndex)); + //std::cout << "Add range select "<< expr.left().syntax->toString() << " : " << expr.right().syntax->toString() << "\n"; } + void addMemberAccess(std::string_view name) { selectors.emplace_back(std::make_unique(name)); } diff --git a/tools/netlist/include/NetlistVisitor.h b/tools/netlist/include/NetlistVisitor.h index f74106cb6..f54dc9b43 100644 --- a/tools/netlist/include/NetlistVisitor.h +++ b/tools/netlist/include/NetlistVisitor.h @@ -76,14 +76,15 @@ class VariableReferenceVisitor : public ast::ASTVisitorkind == ast::ExpressionKind::ElementSelect) { - auto index = selector->as().selector().eval(evalCtx); - node.addElementSelect(index); + const auto& expr = selector->as(); + auto index = expr.selector().eval(evalCtx); + node.addElementSelect(expr, index); } else if (selector->kind == ast::ExpressionKind::RangeSelect) { - auto& rangeSelectExpr = selector->as(); - auto leftIndex = rangeSelectExpr.left().eval(evalCtx); - auto rightIndex = rangeSelectExpr.right().eval(evalCtx); - node.addRangeSelect(leftIndex, rightIndex, rangeSelectExpr.getSelectionKind()); + const auto& expr = selector->as(); + auto leftIndex = expr.left().eval(evalCtx); + auto rightIndex = expr.right().eval(evalCtx); + node.addRangeSelect(expr, leftIndex, rightIndex); } else if (selector->kind == ast::ExpressionKind::MemberAccess) { node.addMemberAccess(selector->as().member.name); diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 64002baa4..29b79bcaf 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -224,7 +224,7 @@ class AnalyseVariableReference { return handleScalarElementSelect(type, range); } else if (selectorsIt->get()->isRangeSelect()) { - switch (selectorsIt->get()->as().selectionKind) { + switch (selectorsIt->get()->as().expr.getSelectionKind()) { case ast::RangeSelectionKind::Simple: return handleScalarRangeSelect(type, range); case ast::RangeSelectionKind::IndexedUp: @@ -249,7 +249,7 @@ class AnalyseVariableReference { return handleArrayElementSelect(type, range); } else if (selectorsIt->get()->isRangeSelect()) { - switch (selectorsIt->get()->as().selectionKind) { + switch (selectorsIt->get()->as().expr.getSelectionKind()) { case ast::RangeSelectionKind::Simple: return handleArrayRangeSelect(type, range); case ast::RangeSelectionKind::IndexedUp: diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index d068f7be4..594d1cc0d 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -38,6 +38,8 @@ module m (input int a); foo[3:1][2:1][1] = 0; foo[7:4][6:5][5] = 0; foo[a] = 0; + foo[a+:2][a] = 0; + foo[a+:2][a+:2][a] = 0; end endmodule )"); @@ -45,6 +47,12 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); + //For (auto &node : netlist) { + // if (node->kind == NodeKind::VariableReference) + // { + // std::cout << "node " << node->as().toString()<<"\n"; + // } + //} CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 31)); CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); @@ -56,8 +64,7 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(5, 6)); CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(5, 5)); - // Can't lookup foo[a] in the netlist. - //CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 31)); } TEST_CASE("Packed 1D array element and range") { From f071ed9cd7395acd5f8d92bea99e49d00b4ce5cc Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 16 Sep 2023 14:39:51 +0100 Subject: [PATCH 16/31] Fix accessors for scalar values --- tools/netlist/include/Netlist.h | 8 ++-- tools/netlist/include/SplitVariables.h | 41 +++++++++++++++---- .../netlist/tests/VariableSelectorsTests.cpp | 19 +++++---- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index c8b4b4436..47299b57d 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -67,10 +67,10 @@ struct VariableSelectorBase { /// A variable selector representing an element selector. struct VariableElementSelect : public VariableSelectorBase { - const ast::ElementSelectExpression &expr; + const ast::Expression &expr; ConstantValue index; - VariableElementSelect(ast::ElementSelectExpression const& expr, ConstantValue index) : + VariableElementSelect(ast::Expression const& expr, ConstantValue index) : VariableSelectorBase(VariableSelectorKind::ElementSelect), expr(expr), index(std::move(index)) {} @@ -272,14 +272,12 @@ class NetlistVariableReference : public NetlistNode { expression(expr), leftOperand(leftOperand) {} void addElementSelect(ast::ElementSelectExpression const &expr, const ConstantValue& index) { - selectors.emplace_back(std::make_unique(expr, index)); - //std::cout << "Add elem select "<< expr.syntax->toString() << "\n"; + selectors.emplace_back(std::make_unique(expr.selector(), index)); } void addRangeSelect(ast::RangeSelectExpression const& expr, const ConstantValue& leftIndex, const ConstantValue& rightIndex) { selectors.emplace_back(std::make_unique(expr, leftIndex, rightIndex)); - //std::cout << "Add range select "<< expr.left().syntax->toString() << " : " << expr.right().syntax->toString() << "\n"; } void addMemberAccess(std::string_view name) { diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 29b79bcaf..9f495db82 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -94,7 +94,7 @@ class AnalyseVariableReference { if (!elementSelector.indexIsConstant()) { // If the selector is not a constant, then return the whole scalar as // the range. - return {range.lower(), (int32_t) type.getBitWidth()}; + return {range.lower(), (int32_t) type.getBitWidth() - 1}; } SLANG_ASSERT(elementSelector.getIndexInt() >= range.lower()); SLANG_ASSERT(elementSelector.getIndexInt() <= range.upper()); @@ -106,8 +106,11 @@ class AnalyseVariableReference { const auto& rangeSelector = selectorsIt->get()->as(); int32_t rightIndex = rangeSelector.getRightIndexInt(); int32_t leftIndex = rangeSelector.getLeftIndexInt(); - //if (!rangeSelector.leftIndexIsConstant()) { - //} + // Left and right indices must be constant. + SLANG_ASSERT(rangeSelector.leftIndexIsConstant()); + SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); + // Assert left and right index values make sense and create the new + // range. SLANG_ASSERT(rightIndex <= leftIndex); SLANG_ASSERT(rightIndex >= range.lower()); SLANG_ASSERT(leftIndex <= range.upper()); @@ -121,10 +124,30 @@ class AnalyseVariableReference { } } - ConstantRange handleScalarRangeSelectUp(const slang::ast::Type &type, ConstantRange range) { - } - - ConstantRange handleScalarRangeSelectDown(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleScalarRangeSelectIncr(const slang::ast::Type &type, ConstantRange range, bool isUp) { + const auto& rangeSelector = selectorsIt->get()->as(); + if (!rangeSelector.leftIndexIsConstant()) { + // If the selector base is not constant, then return the whole scalar + // as the range. + return {range.lower(), (int32_t) type.getBitWidth() - 1}; + } + int32_t rightIndex = rangeSelector.getRightIndexInt(); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); + // Right index must be constant. + SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); + // Assert left and right index values make sense and create the new + // range. + auto rangeEnd = isUp ? rightIndex + leftIndex : rightIndex - leftIndex; + SLANG_ASSERT(rightIndex >= range.lower()); + SLANG_ASSERT(rangeEnd <= range.upper()); + ConstantRange newRange = {range.lower() + rightIndex, + range.lower() + rangeEnd}; + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + return getBitRangeImpl(type, newRange); + } else { + return newRange; + } } ConstantRange handleArrayElementSelect(const slang::ast::Type &type, ConstantRange range) { @@ -228,9 +251,9 @@ class AnalyseVariableReference { case ast::RangeSelectionKind::Simple: return handleScalarRangeSelect(type, range); case ast::RangeSelectionKind::IndexedUp: - return handleScalarRangeSelectUp(type, range); + return handleScalarRangeSelectIncr(type, range, true); case ast::RangeSelectionKind::IndexedDown: - return handleScalarRangeSelectDown(type, range); + return handleScalarRangeSelectIncr(type, range, false); default: SLANG_UNREACHABLE; } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 594d1cc0d..5d2af2d54 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -38,8 +38,11 @@ module m (input int a); foo[3:1][2:1][1] = 0; foo[7:4][6:5][5] = 0; foo[a] = 0; - foo[a+:2][a] = 0; - foo[a+:2][a+:2][a] = 0; + foo[a+:1][a] = 0; + foo[a-:1][a] = 0; + foo[a+:1][a-:1][a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; end endmodule )"); @@ -47,12 +50,6 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - //For (auto &node : netlist) { - // if (node->kind == NodeKind::VariableReference) - // { - // std::cout << "node " << node->as().toString()<<"\n"; - // } - //} CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 31)); CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); @@ -64,7 +61,13 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(5, 6)); CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(5, 5)); + // Dynamic indices. CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a+:1][a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a-:1][a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a+:1][a-:1][a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 31)); } TEST_CASE("Packed 1D array element and range") { From 8214fab42f3b7e52a8b0431a8d48dbc5eab9dfe3 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 16 Sep 2023 14:50:09 +0100 Subject: [PATCH 17/31] Add handling of arrays --- tools/netlist/include/SplitVariables.h | 12 ++++----- .../netlist/tests/VariableSelectorsTests.cpp | 27 +++++++++++++++---- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index 9f495db82..cfca5bded 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -155,7 +155,7 @@ class AnalyseVariableReference { if (!elementSelector.indexIsConstant()) { // If the selector is not a constant, then return the whole scalar as // the range. - return {range.lower(), (int32_t) type.getBitWidth()}; + return {range.lower(), (int32_t) type.getBitWidth() - 1}; } int32_t index = elementSelector.getIndexInt(); auto arrayRange = getArrayRange(type); @@ -196,10 +196,8 @@ class AnalyseVariableReference { } } - ConstantRange handleArrayRangeSelectUp(const slang::ast::Type &type, ConstantRange range) { - } - - ConstantRange handleArrayRangeSelectDown(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleArrayRangeSelectIncr(const slang::ast::Type &type, ConstantRange range, bool isUp) { + return {0,0}; } ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { @@ -276,9 +274,9 @@ class AnalyseVariableReference { case ast::RangeSelectionKind::Simple: return handleArrayRangeSelect(type, range); case ast::RangeSelectionKind::IndexedUp: - return handleArrayRangeSelectUp(type, range); + return handleArrayRangeSelectIncr(type, range, true); case ast::RangeSelectionKind::IndexedDown: - return handleArrayRangeSelectDown(type, range); + return handleArrayRangeSelectIncr(type, range, false); default: SLANG_UNREACHABLE; } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 5d2af2d54..f5f850c6f 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -38,11 +38,12 @@ module m (input int a); foo[3:1][2:1][1] = 0; foo[7:4][6:5][5] = 0; foo[a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; foo[a+:1][a] = 0; foo[a-:1][a] = 0; + foo[a+:1][a-:1] = 0; foo[a+:1][a-:1][a] = 0; - foo[a+:1] = 0; - foo[a-:1] = 0; end endmodule )"); @@ -63,16 +64,17 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(5, 5)); // Dynamic indices. CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 31)); CHECK(getBitRange(netlist, "foo[a+:1][a]") == ConstantRange(0, 31)); CHECK(getBitRange(netlist, "foo[a-:1][a]") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[a+:1][a-:1]") == ConstantRange(0, 31)); CHECK(getBitRange(netlist, "foo[a+:1][a-:1][a]") == ConstantRange(0, 31)); - CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 31)); - CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 31)); } TEST_CASE("Packed 1D array element and range") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic [3:0] foo; always_comb begin foo = 0; @@ -85,6 +87,13 @@ module m; foo[3:0] = 0; foo[2:1] = 0; foo[3:1][2:1][1] = 0; + foo[a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; + foo[a+:1][a] = 0; + foo[a-:1][a] = 0; + foo[a+:1][a-:1] = 0; + foo[a+:1][a-:1][a] = 0; end endmodule )"); @@ -102,6 +111,14 @@ endmodule CHECK(getBitRange(netlist, "foo[3:0]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[2:1]") == ConstantRange(1, 2)); CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 3)); + //CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 3)); + //CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a+:1][a]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a-:1][a]") == ConstantRange(0, 3)); + //CHECK(getBitRange(netlist, "foo[a+:1][a-:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a+:1][a-:1][a]") == ConstantRange(0, 3)); } TEST_CASE("Packed 1D array element and range non-zero indexed") { From 6281e3450737c385ec7962f4d4996ee3bb271449 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 16 Sep 2023 21:00:18 +0100 Subject: [PATCH 18/31] Complete test cases for element and range select --- tools/netlist/TODO.md | 36 +++++++- tools/netlist/include/SplitVariables.h | 55 +++++++++--- .../netlist/tests/VariableSelectorsTests.cpp | 87 +++++++++++++++++-- 3 files changed, 157 insertions(+), 21 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index da2bb96b9..d33913788 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -3,8 +3,40 @@ To dos - Bug report: - In ast::ElementSelectExpr.syntax does not correspond to selector source - range and includes entire variable reference + selectors. + module m (input int a); + logic [3:0] [1:0] foo; + always_comb begin + foo[a+:1][1] = 0; + foo[a-:1][1] = 0; + end + endmodule + +/home/jamie/slang/tools/netlist/tests/VariableSelectorsTests.cpp:198: FAILED: +explicitly with message: + source:23:15: warning: cannot refer to element 1 of 'logic[0:0][1:0]' [- + Windex-oob] + foo[a+:1][1] = 0; + ^ + source:24:15: warning: cannot refer to element 1 of 'logic[3:3][1:0]' [- + Windex-oob] + foo[a-:1][1] = 0; + +Similar, baz and bat assignents not valid: + + module m (input int a); + logic foo [1:0]; + logic bar [1:0]; + logic baz [0:0]; + logic bat [1:1]; + always_comb begin + foo = bar; + foo[0] = 0; + foo[1] = 0; + foo[a] = 0; + foo[a+:1] = baz; + foo[a-:1] = bat; + end + endmodule - Bug report: diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index cfca5bded..efa59cb72 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -69,7 +69,7 @@ class AnalyseVariableReference { if (member.name == fieldName) { return {(int32_t) offset, (int32_t) offset + fieldWidth}; }; - offset += fieldWidth;; + offset += fieldWidth; } SLANG_UNREACHABLE; } @@ -93,8 +93,9 @@ class AnalyseVariableReference { const auto& elementSelector = selectorsIt->get()->as(); if (!elementSelector.indexIsConstant()) { // If the selector is not a constant, then return the whole scalar as - // the range. - return {range.lower(), (int32_t) type.getBitWidth() - 1}; + // the range. No further selectors expected. + SLANG_ASSERT(std::next(selectorsIt) == node.selectors.end()); + return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; } SLANG_ASSERT(elementSelector.getIndexInt() >= range.lower()); SLANG_ASSERT(elementSelector.getIndexInt() <= range.upper()); @@ -128,13 +129,14 @@ class AnalyseVariableReference { const auto& rangeSelector = selectorsIt->get()->as(); if (!rangeSelector.leftIndexIsConstant()) { // If the selector base is not constant, then return the whole scalar - // as the range. - return {range.lower(), (int32_t) type.getBitWidth() - 1}; + // as the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; } - int32_t rightIndex = rangeSelector.getRightIndexInt(); - int32_t leftIndex = rangeSelector.getLeftIndexInt(); // Right index must be constant. SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); + int32_t rightIndex = rangeSelector.getRightIndexInt(); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); // Assert left and right index values make sense and create the new // range. auto rangeEnd = isUp ? rightIndex + leftIndex : rightIndex - leftIndex; @@ -154,8 +156,9 @@ class AnalyseVariableReference { const auto& elementSelector = selectorsIt->get()->as(); if (!elementSelector.indexIsConstant()) { // If the selector is not a constant, then return the whole scalar as - // the range. - return {range.lower(), (int32_t) type.getBitWidth() - 1}; + // the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), range.lower() + getTypeBitWidth(type) - 1}; } int32_t index = elementSelector.getIndexInt(); auto arrayRange = getArrayRange(type); @@ -179,6 +182,10 @@ class AnalyseVariableReference { int32_t leftIndex = rangeSelector.getLeftIndexInt(); int32_t rightIndex = rangeSelector.getRightIndexInt(); auto arrayRange = getArrayRange(type); + // Left and right indices must be constant. + SLANG_ASSERT(rangeSelector.leftIndexIsConstant()); + SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); + // Assert left and right index values make sense. SLANG_ASSERT(rightIndex >= arrayRange.lower()); SLANG_ASSERT(leftIndex <= arrayRange.upper()); SLANG_ASSERT(rightIndex <= leftIndex); @@ -197,7 +204,35 @@ class AnalyseVariableReference { } ConstantRange handleArrayRangeSelectIncr(const slang::ast::Type &type, ConstantRange range, bool isUp) { - return {0,0}; + const auto& rangeSelector = selectorsIt->get()->as(); + auto* elementType = type.getArrayElementType(); + auto arrayRange = getArrayRange(type); + if (!rangeSelector.leftIndexIsConstant()) { + // If the selector base is not constant, then return the whole array + // as the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), + range.lower() + (getTypeBitWidth(*elementType) * (int32_t)arrayRange.width()) - 1}; + } + // Right index must be constant. + SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); + int32_t rightIndex = rangeSelector.getRightIndexInt(); + int32_t leftIndex = rangeSelector.getLeftIndexInt(); + // Assert left and right index values make sense. + SLANG_ASSERT(rightIndex >= arrayRange.lower()); + SLANG_ASSERT(leftIndex <= arrayRange.upper()); + SLANG_ASSERT(rightIndex <= leftIndex); + // Adjust for non-zero array indexing. + leftIndex -= arrayRange.lower(); + rightIndex -= arrayRange.lower(); + ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), + range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + return getBitRangeImpl(type, newRange); + } else { + return newRange; + } } ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index f5f850c6f..34baf1751 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -113,17 +113,17 @@ endmodule CHECK(getBitRange(netlist, "foo[3:1][2:1][1]") == ConstantRange(1, 1)); // Dynamic indices. CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 3)); - //CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 3)); - //CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[a+:1][a]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[a-:1][a]") == ConstantRange(0, 3)); - //CHECK(getBitRange(netlist, "foo[a+:1][a-:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a+:1][a-:1]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[a+:1][a-:1][a]") == ConstantRange(0, 3)); } TEST_CASE("Packed 1D array element and range non-zero indexed") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic [7:4] foo; always_comb begin foo = 0; @@ -136,6 +136,9 @@ module m; foo[7:4] = 0; foo[6:5] = 0; foo[7:4][6:5][5] = 0; + foo[a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; end endmodule )"); @@ -153,11 +156,15 @@ endmodule CHECK(getBitRange(netlist, "foo[7:4]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[6:5]") == ConstantRange(1, 2)); CHECK(getBitRange(netlist, "foo[7:4][6:5][5]") == ConstantRange(1, 1)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 3)); } TEST_CASE("Packed 2D array element and range") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic [3:0] [1:0] foo; always_comb begin foo = 0; @@ -173,6 +180,16 @@ module m; foo[3:2] = 0; foo[3:0][2:1] = 0; foo[3:0][2:1][1] = 0; + foo[a] = 0; + foo[a][1] = 0; + foo[a][a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; + //foo[a+:1][1] = 0; + //foo[a-:1][1] = 0; + foo[1][a] = 0; + foo[1][a+:1] = 0; + foo[1][a-:1] = 0; end endmodule )"); @@ -193,11 +210,22 @@ endmodule CHECK(getBitRange(netlist, "foo[3:2]") == ConstantRange(4, 7)); CHECK(getBitRange(netlist, "foo[3:0][2:1]") == ConstantRange(2, 5)); CHECK(getBitRange(netlist, "foo[3:0][2:1][1]") == ConstantRange(2, 3)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a][1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a][a]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 7)); + //CHECK(getBitRange(netlist, "foo[a+:1][1]") == ConstantRange(0, 7)); + //CHECK(getBitRange(netlist, "foo[a-:1][1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[1][a]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[1][a+:1]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[1][a-:1]") == ConstantRange(2, 3)); } TEST_CASE("Packed 2D array element and range, non-zero indexing") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic [7:4] [3:2] foo; always_comb begin foo = 0; @@ -206,6 +234,12 @@ module m; foo[5:4] = 0; foo[7:4][6:5] = 0; foo[7:5][6:5][5] = 0; + foo[a] = 0; + foo[a+:1] = 0; + foo[a-:1] = 0; + foo[5][a] = 0; + foo[5][a+:1] = 0; + foo[5][a-:1] = 0; end endmodule )"); @@ -219,17 +253,27 @@ endmodule CHECK(getBitRange(netlist, "foo[5:4]") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo[7:4][6:5]") == ConstantRange(2, 5)); CHECK(getBitRange(netlist, "foo[7:5][6:5][5]") == ConstantRange(2, 3)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[5][a]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[5][a+:1]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[5][a-:1]") == ConstantRange(2, 3)); } TEST_CASE("Unpacked 1D array element") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic foo [1:0]; logic bar [1:0]; always_comb begin foo = bar; foo[0] = 0; foo[1] = 0; + foo[a] = 0; + foo[a+:1] = '{0}; + foo[a-:2] = '{0, 0}; end endmodule )"); @@ -240,11 +284,15 @@ endmodule CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 1)); CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 0)); CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 1)); + CHECK(getBitRange(netlist, "foo[a-:2]") == ConstantRange(0, 1)); } -TEST_CASE("Unpacked 2D array element") { +TEST_CASE("Unpacked 2D array element and range") { auto tree = SyntaxTree::fromText(R"( -module m; +module m (input int a); logic foo [3:0] [1:0]; logic bar [1:0]; always_comb begin @@ -256,6 +304,16 @@ module m; foo[1][1] = 0; foo[2][1] = 0; foo[3][1] = 0; + foo[a] = bar; + foo[a][1] = 0; + foo[a][a] = 0; + foo[a+:1] = '{'{0, 0}}; + foo[a-:2] = '{'{0, 0}, '{0, 0}}; + //foo[a+:1][1] = 0; + //foo[a-:1][1] = 0; + foo[1][a] = 0; + foo[1][a+:1] = '{0}; + foo[1][a-:2] = '{0, 0}; end endmodule )"); @@ -269,6 +327,17 @@ endmodule CHECK(getBitRange(netlist, "foo[1][1]") == ConstantRange(3, 3)); CHECK(getBitRange(netlist, "foo[2][1]") == ConstantRange(5, 5)); CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(7, 7)); + // Dynamic indices. + CHECK(getBitRange(netlist, "foo[a]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a][1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a][a]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[a-:2]") == ConstantRange(0, 7)); + //CHECK(getBitRange(netlist, "foo[a+:1][1]") == ConstantRange(0, 7)); + //CHECK(getBitRange(netlist, "foo[a-:1][1]") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[1][a]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[1][a+:1]") == ConstantRange(2, 3)); + CHECK(getBitRange(netlist, "foo[1][a-:2]") == ConstantRange(2, 3)); } //===---------------------------------------------------------------------===// From 351170a4940e3a6e96f269094869ce349c4e282a Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 16 Sep 2023 21:12:34 +0100 Subject: [PATCH 19/31] Reenable unit tests --- tools/netlist/tests/CMakeLists.txt | 2 +- tools/netlist/tests/NetlistTests.cpp | 257 ------------------ .../netlist/tests/VariableSelectorsTests.cpp | 254 +---------------- 3 files changed, 13 insertions(+), 500 deletions(-) delete mode 100644 tools/netlist/tests/NetlistTests.cpp diff --git a/tools/netlist/tests/CMakeLists.txt b/tools/netlist/tests/CMakeLists.txt index 48268113b..f4ecb2cdf 100644 --- a/tools/netlist/tests/CMakeLists.txt +++ b/tools/netlist/tests/CMakeLists.txt @@ -7,7 +7,7 @@ add_executable( netlist_unittests ../../../tests/unittests/main.cpp ../../../tests/unittests/Test.cpp ../source/Netlist.cpp - DepthFirstSearchTests.cpp DirectedGraphTests.cpp NetlistTests.cpp + DepthFirstSearchTests.cpp DirectedGraphTests.cpp NameTests.cpp PathTests.cpp VariableSelectorsTests.cpp) target_link_libraries( diff --git a/tools/netlist/tests/NetlistTests.cpp b/tools/netlist/tests/NetlistTests.cpp deleted file mode 100644 index 19cc68522..000000000 --- a/tools/netlist/tests/NetlistTests.cpp +++ /dev/null @@ -1,257 +0,0 @@ -//------------------------------------------------------------------------------ -//! @file NetlistTest.cpp -//! @brief Netlist unit tests. -// -// SPDX-FileCopyrightText: Michael Popoloski -// SPDX-License-Identifier: MIT -//------------------------------------------------------------------------------ - -#include "NetlistTest.h" - -//===---------------------------------------------------------------------===// -// Basic tests -//===---------------------------------------------------------------------===// - -TEST_CASE("Empty module") { - // Test the simplest path can be traced through a module. - auto tree = SyntaxTree::fromText(R"( -module empty (input logic i_value, output logic o_value); -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(netlist.numNodes() == 4); - CHECK(netlist.numEdges() == 2); - // Lookup the two ports in the netlist. - auto* inPort = netlist.lookupPort("empty.i_value"); - CHECK(inPort != nullptr); - auto* outPort = netlist.lookupPort("empty.o_value"); - CHECK(outPort != nullptr); -} - -TEST_CASE("Pass through a module") { - // Test the simplest path through a module. - auto tree = SyntaxTree::fromText(R"( -module passthrough (input logic i_value, output logic o_value); - assign o_value = i_value; -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(netlist.numNodes() == 6); - CHECK(netlist.numEdges() == 5); - // Lookup the two ports in the netlist. - auto* inPort = netlist.lookupPort("passthrough.i_value"); - CHECK(inPort != nullptr); - auto* outPort = netlist.lookupPort("passthrough.o_value"); - CHECK(outPort != nullptr); - PathFinder pathFinder(netlist); - // Valid i_value -> o_value - CHECK(!pathFinder.find(*inPort, *outPort).empty()); - // Invalid o_value -> i_value - CHECK(pathFinder.find(*outPort, *inPort).empty()); -} - -TEST_CASE("Chain of assignments in a sequence using variables") { - // Test that correct dependencies can be formed from procedural and - // continuous assignments. - auto tree = SyntaxTree::fromText(R"( -module chain_vars (input logic i_value, output logic o_value); - - logic a, b, c, d, e; - - assign a = i_value; - - always_comb begin - b = a; - c = b; - d = c; - end - - assign e = d; - assign o_value = e; - -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(netlist.numNodes() == 21); - CHECK(netlist.numEdges() == 20); - PathFinder pathFinder(netlist); - // Find path i_value -> o_value - auto path = pathFinder.find(*netlist.lookupPort("chain_vars.i_value"), - *netlist.lookupPort("chain_vars.o_value")); - CHECK(path.size() == 12); - CHECK(*path.findVariable("chain_vars.a") == 1); - CHECK(*path.findVariable("chain_vars.b") == 3); - CHECK(*path.findVariable("chain_vars.c") == 5); - CHECK(*path.findVariable("chain_vars.d") == 7); - CHECK(*path.findVariable("chain_vars.e") == 9); -} - -//===---------------------------------------------------------------------===// -// Tests for module instance connectivity. -//===---------------------------------------------------------------------===// - -TEST_CASE("Signal passthrough with a nested module") { - // Test that a nested module is connected correctly. - auto tree = SyntaxTree::fromText(R"( -module passthrough(input logic i_value, output logic o_value); - assign o_value = i_value; -endmodule - -module nested_passthrough(input logic i_value, output logic o_value); - passthrough foo( - .i_value(i_value), - .o_value(o_value)); -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - auto path = pathFinder.find(*netlist.lookupPort("nested_passthrough.i_value"), - *netlist.lookupPort("nested_passthrough.o_value")); - CHECK(*path.findVariable("nested_passthrough.foo.i_value") == 1); - CHECK(*path.findVariable("nested_passthrough.foo.o_value") == 2); -} - -TEST_CASE("Signal passthrough with a chain of two nested modules") { - // Test that two nested module are connected correctly. - auto tree = SyntaxTree::fromText(R"( -module passthrough(input logic i_value, output logic o_value); - assign o_value = i_value; -endmodule - -module nested_passthrough(input logic i_value, output logic o_value); - logic value; - passthrough foo_a( - .i_value(i_value), - .o_value(value)); - passthrough foo_b( - .i_value(value), - .o_value(o_value)); -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - auto path = pathFinder.find(*netlist.lookupPort("nested_passthrough.i_value"), - *netlist.lookupPort("nested_passthrough.o_value")); - CHECK(path.size() == 8); - CHECK(*path.findVariable("nested_passthrough.foo_a.i_value") == 1); - CHECK(*path.findVariable("nested_passthrough.foo_a.o_value") == 2); - CHECK(*path.findVariable("nested_passthrough.foo_b.i_value") == 5); - CHECK(*path.findVariable("nested_passthrough.foo_b.o_value") == 6); -} - -//===---------------------------------------------------------------------===// -// Tests for conditional variables in procedural blocks. -//===---------------------------------------------------------------------===// - -TEST_CASE("Mux") { - // Test that the variable in a conditional block is correctly added as a - // dependency on the output variable controlled by that block. - auto tree = SyntaxTree::fromText(R"( -module mux(input a, input b, input sel, output reg f); - always @(*) begin - if (sel == 1'b0) begin - f = a; - end else begin - f = b; - end - end -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel"), *netlist.lookupPort("mux.f")).empty()); -} - -TEST_CASE("Nested muxing") { - // Test that the variables in multiple nested levels of conditions are - // correctly added as dependencies of the output variable. - auto tree = SyntaxTree::fromText(R"( -module mux(input a, input b, input c, - input sel_a, input sel_b, - output reg f); - always @(*) begin - if (sel_a == 1'b0) begin - if (sel_b == 1'b0) - f = a; - else - f = b; - end else begin - f = c; - end - end -endmodule -)"); - Compilation compilation; - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - PathFinder pathFinder(netlist); - CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_a"), *netlist.lookupPort("mux.f")).empty()); - CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_b"), *netlist.lookupPort("mux.f")).empty()); -} - -//===---------------------------------------------------------------------===// -// Tests for name resolution -//===---------------------------------------------------------------------===// - -TEST_CASE("Unused modules") { - // Test that unused modules are not visited by the netlist builder. - // See Issue #793. - auto tree = SyntaxTree::fromText(R"( -module test (input i1, - input i2, - output o1 - ); - cell_a i_cell_a(.d1(i1), - .d2(i2), - .c(o1)); -endmodule - -module cell_a(input d1, - input d2, - output c); - assign c = d1 + d2; -endmodule - -// unused -module cell_b(input a, - input b, - output z); - assign z = a || b; -endmodule - -// unused -module cell_c(input a, - input b, - output z); - assign z = (!a) && b; -endmodule -)"); - CompilationOptions coptions; - coptions.topModules.emplace("test"sv); - Bag options; - options.set(coptions); - Compilation compilation(options); - compilation.addSyntaxTree(tree); - NO_COMPILATION_ERRORS; - auto netlist = createNetlist(compilation); - CHECK(netlist.numNodes() > 0); -} diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 34baf1751..3c6809792 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -21,6 +21,10 @@ ConstantRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { return AnalyseVariableReference::create(*node).getBitRange(); } +//===---------------------------------------------------------------------===// +// Scalar selectors. +//===---------------------------------------------------------------------===// + TEST_CASE("Scalar element and range") { auto tree = SyntaxTree::fromText(R"( module m (input int a); @@ -72,6 +76,10 @@ endmodule CHECK(getBitRange(netlist, "foo[a+:1][a-:1][a]") == ConstantRange(0, 31)); } +//===---------------------------------------------------------------------===// +// Packed array selectors. +//===---------------------------------------------------------------------===// + TEST_CASE("Packed 1D array element and range") { auto tree = SyntaxTree::fromText(R"( module m (input int a); @@ -262,6 +270,10 @@ endmodule CHECK(getBitRange(netlist, "foo[5][a-:1]") == ConstantRange(2, 3)); } +//===---------------------------------------------------------------------===// +// Unpacked array selectors. +//===---------------------------------------------------------------------===// + TEST_CASE("Unpacked 1D array element") { auto tree = SyntaxTree::fromText(R"( module m (input int a); @@ -340,245 +352,3 @@ endmodule CHECK(getBitRange(netlist, "foo[1][a-:2]") == ConstantRange(2, 3)); } -//===---------------------------------------------------------------------===// -// Tests for variable splitting -//===---------------------------------------------------------------------===// - -//TEST_CASE("Chain of assignments in a sequence using a vector") { -// // As above but this time using a packed array. -// auto tree = SyntaxTree::fromText(R"( -//module chain_array (input logic i_value, output logic o_value); -// -// logic [4:0] x; -// -// assign x[0] = i_value; -// -// always_comb begin -// x[1] = x[0]; -// x[2] = x[1]; -// x[3] = x[2]; -// end -// -// assign x[4] = x[3]; -// assign o_value = x[4]; -// -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -// PathFinder pathFinder(netlist); -// auto path = pathFinder.find(*netlist.lookupPort("chain_array.i_value"), -// *netlist.lookupPort("chain_array.o_value")); -// CHECK(*path.findVariable("chain_array.x[0]") == 1); -// CHECK(*path.findVariable("chain_array.x[1]") == 3); -// CHECK(*path.findVariable("chain_array.x[2]") == 5); -// CHECK(*path.findVariable("chain_array.x[3]") == 7); -// CHECK(*path.findVariable("chain_array.x[4]") == 9); -//} -// -//TEST_CASE("Passthrough two signals via a shared structure") { -// auto tree = SyntaxTree::fromText(R"( -//module passthrough_member_access ( -// input logic i_value_a, -// input logic i_value_b, -// output logic o_value_a, -// output logic o_value_b -//); -// -// struct packed { -// logic a; -// logic b; -// } foo; -// -// assign foo.a = i_value_a; -// assign foo.b = i_value_b; -// -// assign o_value_a = foo.a; -// assign o_value_b = foo.b; -// -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -// auto* inPortA = netlist.lookupPort("passthrough_member_access.i_value_a"); -// auto* inPortB = netlist.lookupPort("passthrough_member_access.i_value_b"); -// auto* outPortA = netlist.lookupPort("passthrough_member_access.o_value_a"); -// auto* outPortB = netlist.lookupPort("passthrough_member_access.o_value_b"); -// PathFinder pathFinder(netlist); -// // Valid paths. -// CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); -// CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); -// // Invalid paths. -// CHECK(pathFinder.find(*inPortA, *outPortB).empty()); -// CHECK(pathFinder.find(*inPortB, *outPortA).empty()); -//} -// -//TEST_CASE("Passthrough two signals via ranges in a shared vector") { -// auto tree = SyntaxTree::fromText(R"( -//module passthrough_ranges ( -// input logic [1:0] i_value_a, -// input logic [1:0] i_value_b, -// output logic [1:0] o_value_a, -// output logic [1:0] o_value_b -//); -// -// logic [3:0] foo; -// -// assign foo[1:0] = i_value_a; -// assign foo[3:2] = i_value_b; -// -// assign o_value_a = foo[1:0]; -// assign o_value_b = foo[3:2]; -// -//endmodule -//)"); -// Compilation compilation; -// compilation.addSyntaxTree(tree); -// NO_COMPILATION_ERRORS; -// auto netlist = createNetlist(compilation); -// auto* inPortA = netlist.lookupPort("passthrough_ranges.i_value_a"); -// auto* inPortB = netlist.lookupPort("passthrough_ranges.i_value_b"); -// auto* outPortA = netlist.lookupPort("passthrough_ranges.o_value_a"); -// auto* outPortB = netlist.lookupPort("passthrough_ranges.o_value_b"); -// PathFinder pathFinder(netlist); -// // Valid paths. -// CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); -// CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); -// // Invalid paths. -// CHECK(pathFinder.find(*inPortA, *outPortB).empty()); -// CHECK(pathFinder.find(*inPortB, *outPortA).empty()); -//} - -//===---------------------------------------------------------------------===// -// Tests for loop unrolling -//===---------------------------------------------------------------------===// - -//TEST_CASE("Chain of assignments using a single loop") { -// // Test that a loop can be unrolled and variable declarations can be split -// // out for elements of an array. -// auto tree = SyntaxTree::fromText(R"( -//module chain_array #(parameter N=4) ( -// input logic i_value, -// output logic o_value); -// -// logic pipeline [N-1:0]; -// -// assign pipeline[0] = i_value; -// -// always_comb begin -// for (int i=1; i o_value, check it passes through each stage. -// CHECK(pathFinder -// .find(*netlist.lookupPort("chain_array.i_value"), -// *netlist.lookupPort("chain_array.o_value")) -// .size() == 10); -//} -// -//TEST_CASE("Chain of assignments using a nested loop") { -// // Expand the previous test to check the handling of multiple loop variables. -// auto tree = SyntaxTree::fromText(R"( -//module chain_nested #(parameter N=3) ( -// input logic i_value, -// output logic o_value); -// -// logic [(N*N)-1:0] stages; -// -// //assign stages[0] = i_value; -// assign o_value = stages[(N*N)-1]; -// -// always_comb begin -// for (int i=0; i o_value, check it passes through each stage. -// auto path = pathFinder.find(*netlist.lookupPort("chain_nested.i_value"), -// *netlist.lookupPort("chain_nested.o_value")); -// CHECK(*path.findVariable("chain_nested.stages[0]") == 1); -// CHECK(*path.findVariable("chain_nested.stages[1]") == 3); -// CHECK(*path.findVariable("chain_nested.stages[2]") == 5); -// CHECK(*path.findVariable("chain_nested.stages[3]") == 7); -// CHECK(*path.findVariable("chain_nested.stages[4]") == 9); -// CHECK(*path.findVariable("chain_nested.stages[5]") == 11); -// CHECK(*path.findVariable("chain_nested.stages[6]") == 13); -// CHECK(*path.findVariable("chain_nested.stages[7]") == 15); -// CHECK(*path.findVariable("chain_nested.stages[8]") == 17); -//} -// -//TEST_CASE("Two chains of assignments using a shared 2D array") { -// // Check that assignments corresponding to two distinct paths, using the same -// // array variable can be handled. -// auto tree = SyntaxTree::fromText(R"( -//module chain_loop_dual #(parameter N=4)( -// input logic i_value_a, -// input logic i_value_b, -// output logic o_value_a, -// output logic o_value_b -//); -// -// parameter A = 0; -// parameter B = 1; -// -// logic pipeline [1:0][N-1:0]; -// -// assign pipeline[A][0] = i_value_a; -// assign pipeline[B][0] = i_value_b; -// -// always_comb begin -// for (int i=1; i Date: Sat, 16 Sep 2023 22:05:53 +0100 Subject: [PATCH 20/31] Handle unions and enums --- include/slang/numeric/ConstantValue.h | 3 + source/numeric/ConstantValue.cpp | 4 + tools/netlist/include/SplitVariables.h | 118 +++++++++++++++++-------- 3 files changed, 89 insertions(+), 36 deletions(-) diff --git a/include/slang/numeric/ConstantValue.h b/include/slang/numeric/ConstantValue.h index 3d00684be..adcb63b4d 100644 --- a/include/slang/numeric/ConstantValue.h +++ b/include/slang/numeric/ConstantValue.h @@ -345,6 +345,9 @@ struct SLANG_EXPORT ConstantRange { /// Determines whether the given point is within the range. bool containsPoint(int32_t index) const; + /// Determines whether the given range is wholly contained within this one. + bool contains(ConstantRange other) const; + /// Determines whether the given range overlaps with this one /// (including cases where one is wholly contained in the other). bool overlaps(ConstantRange other) const; diff --git a/source/numeric/ConstantValue.cpp b/source/numeric/ConstantValue.cpp index 1656168b6..0074cb929 100644 --- a/source/numeric/ConstantValue.cpp +++ b/source/numeric/ConstantValue.cpp @@ -659,6 +659,10 @@ bool ConstantRange::containsPoint(int32_t index) const { return index >= lower() && index <= upper(); } +bool ConstantRange::contains(ConstantRange other) const { + return other.lower() >= lower() && other.upper() <= upper(); +} + bool ConstantRange::overlaps(ConstantRange other) const { return lower() <= other.upper() && upper() >= other.lower(); } diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index efa59cb72..f3a8be34f 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -60,20 +60,43 @@ class AnalyseVariableReference { return (int32_t) (multiplierElem * fixedSizeElem); } + /// Given a scope, return the type of the named field. + slang::ast::Type const& getScopeFieldType(const slang::ast::Scope& scope, + const std::string_view name) const { + auto* symbol = scope.find(name); + SLANG_ASSERT(symbol != nullptr); + return symbol->getDeclaredType()->getType(); + } + /// Given a packed struct type, return the bit position of the named field. - ConstantRange getFieldRange(const slang::ast::PackedStructType &packedStruct, - const std::string_view fieldName) const { - size_t offset = 0; + ConstantRange getStructFieldRange(const slang::ast::PackedStructType &packedStruct, + const std::string_view fieldName) const { + int32_t offset = 0; for (auto &member : packedStruct.members()) { int32_t fieldWidth = member.getDeclaredType()->getType().getBitWidth(); if (member.name == fieldName) { - return {(int32_t) offset, (int32_t) offset + fieldWidth}; + return {offset, offset + fieldWidth - 1}; }; offset += fieldWidth; } SLANG_UNREACHABLE; } + /// Given a packed union type, return the bit position of the named field. + ConstantRange getUnionFieldRange(const slang::ast::PackedUnionType &packedUnion, + const std::string_view fieldName) const { + auto* symbol = packedUnion.find(fieldName); + SLANG_ASSERT(symbol != nullptr); + int32_t fieldWidth = symbol->getDeclaredType()->getType().getBitWidth(); + return {0, fieldWidth - 1}; + } + + /// Given an enumeration type, return the bit position of the named field. + ConstantRange getEnumRange(const slang::ast::EnumType& enumeration) { + auto fieldRange = enumeration.getBitVectorRange(); + return {0, (int32_t)fieldRange.width() - 1}; + } + /// Given an array type, return the range from which the array is indexed. ConstantRange getArrayRange(const slang::ast::Type &type) { if (type.kind == slang::ast::SymbolKind::PackedArrayType) { @@ -97,9 +120,9 @@ class AnalyseVariableReference { SLANG_ASSERT(std::next(selectorsIt) == node.selectors.end()); return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; } - SLANG_ASSERT(elementSelector.getIndexInt() >= range.lower()); - SLANG_ASSERT(elementSelector.getIndexInt() <= range.upper()); + // Create a new range. int32_t index = range.lower() + elementSelector.getIndexInt(); + SLANG_ASSERT(range.containsPoint(index)); return {index, index}; } @@ -110,13 +133,10 @@ class AnalyseVariableReference { // Left and right indices must be constant. SLANG_ASSERT(rangeSelector.leftIndexIsConstant()); SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); - // Assert left and right index values make sense and create the new - // range. - SLANG_ASSERT(rightIndex <= leftIndex); - SLANG_ASSERT(rightIndex >= range.lower()); - SLANG_ASSERT(leftIndex <= range.upper()); + // Create a new range. ConstantRange newRange = {range.lower() + rightIndex, range.lower() + leftIndex}; + SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -137,13 +157,11 @@ class AnalyseVariableReference { SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); int32_t rightIndex = rangeSelector.getRightIndexInt(); int32_t leftIndex = rangeSelector.getLeftIndexInt(); - // Assert left and right index values make sense and create the new - // range. + // Create a new range. auto rangeEnd = isUp ? rightIndex + leftIndex : rightIndex - leftIndex; - SLANG_ASSERT(rightIndex >= range.lower()); - SLANG_ASSERT(rangeEnd <= range.upper()); ConstantRange newRange = {range.lower() + rightIndex, range.lower() + rangeEnd}; + SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -162,13 +180,14 @@ class AnalyseVariableReference { } int32_t index = elementSelector.getIndexInt(); auto arrayRange = getArrayRange(type); - SLANG_ASSERT(index >= arrayRange.lower()); - SLANG_ASSERT(index <= arrayRange.upper()); + SLANG_ASSERT(arrayRange.containsPoint(index)); // Adjust for non-zero array indexing. index -= arrayRange.lower(); + // Create a new range. auto* elementType = type.getArrayElementType(); ConstantRange newRange = {range.lower() + (index * getTypeBitWidth(*elementType)), range.lower() + ((index + 1) * getTypeBitWidth(*elementType)) - 1}; + SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); @@ -185,16 +204,14 @@ class AnalyseVariableReference { // Left and right indices must be constant. SLANG_ASSERT(rangeSelector.leftIndexIsConstant()); SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); - // Assert left and right index values make sense. - SLANG_ASSERT(rightIndex >= arrayRange.lower()); - SLANG_ASSERT(leftIndex <= arrayRange.upper()); - SLANG_ASSERT(rightIndex <= leftIndex); // Adjust for non-zero array indexing. leftIndex -= arrayRange.lower(); rightIndex -= arrayRange.lower(); + // Create a new range. auto* elementType = type.getArrayElementType(); ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; + SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -218,15 +235,13 @@ class AnalyseVariableReference { SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); int32_t rightIndex = rangeSelector.getRightIndexInt(); int32_t leftIndex = rangeSelector.getLeftIndexInt(); - // Assert left and right index values make sense. - SLANG_ASSERT(rightIndex >= arrayRange.lower()); - SLANG_ASSERT(leftIndex <= arrayRange.upper()); - SLANG_ASSERT(rightIndex <= leftIndex); // Adjust for non-zero array indexing. leftIndex -= arrayRange.lower(); rightIndex -= arrayRange.lower(); + // Create a new range. ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; + SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); @@ -236,25 +251,56 @@ class AnalyseVariableReference { } ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { - //const auto& packedStruct = type.getCanonicalType().as(); - //SLANG_ASSERT(selectorsIt->get()->kind == VariableSelectorKind::MemberAccess); - //const auto& memberAccessSelector = selectorsIt->get()->as(); - //auto fieldRange = getFieldRange(packedStruct, memberAccessSelector.name); - //SLANG_ASSERT(range.contains(fieldRange)); - //auto fieldType = packedStruct.getNameMap()[memberAccessSelector.name]; - //return getBitRange(fieldType, fieldRange.start, fieldRange.end); - return {0, 0}; + const auto& memberAccessSelector = selectorsIt->get()->as(); + const auto& packedStruct = type.getCanonicalType().as(); + auto fieldRange = getStructFieldRange(packedStruct, memberAccessSelector.name); + // Create a new range. + ConstantRange newRange = {range.lower() + fieldRange.lower(), + range.lower() + fieldRange.upper()}; + SLANG_ASSERT(range.contains(newRange)); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + const auto& fieldType = getScopeFieldType(packedStruct, memberAccessSelector.name); + return getBitRangeImpl(fieldType, fieldRange); + } else { + return newRange; + } } ConstantRange handleUnionMemberAccess(const slang::ast::Type &type, ConstantRange range) { - return {0, 0}; + const auto& memberAccessSelector = selectorsIt->get()->as(); + const auto& packedUnion = type.getCanonicalType().as(); + auto fieldRange = getUnionFieldRange(packedUnion, memberAccessSelector.name); + // Create a new range. + ConstantRange newRange = {range.lower() + fieldRange.lower(), + range.lower() + fieldRange.upper()}; + SLANG_ASSERT(range.contains(newRange)); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + const auto& fieldType = getScopeFieldType(packedUnion, memberAccessSelector.name); + return getBitRangeImpl(fieldType, fieldRange); + } else { + return newRange; + } } ConstantRange handleEnumMemberAccess(const slang::ast::Type &type, ConstantRange range) { - return {0, 0}; + const auto& memberAccessSelector = selectorsIt->get()->as(); + const auto& enumeration = type.getCanonicalType().as(); + auto fieldRange = getEnumRange(enumeration); + // Create a new range. + ConstantRange newRange = {range.lower() + fieldRange.lower(), + range.lower() + fieldRange.upper()}; + SLANG_ASSERT(range.contains(newRange)); + if (std::next(selectorsIt) != node.selectors.end()) { + selectorsIt++; + const auto& fieldType = getScopeFieldType(enumeration, memberAccessSelector.name); + return getBitRangeImpl(fieldType, fieldRange); + } else { + return newRange; + } } - // Multiple range selectors have only the effect of the last one. // Eg x[3:0][2:1] <=> x[2:1] or x[2:1][2] <=> x[2]. inline bool ignoreSelector() { From c30f3ebf9c8f69edd269a97ba63466b0a32764cc Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 16 Sep 2023 22:06:14 +0100 Subject: [PATCH 21/31] Add files --- tools/netlist/tests/NameTests.cpp | 58 +++ tools/netlist/tests/PathTests.cpp | 492 ++++++++++++++++++ .../netlist/tests/VariableSelectorsTests.cpp | 8 + 3 files changed, 558 insertions(+) create mode 100644 tools/netlist/tests/NameTests.cpp create mode 100644 tools/netlist/tests/PathTests.cpp diff --git a/tools/netlist/tests/NameTests.cpp b/tools/netlist/tests/NameTests.cpp new file mode 100644 index 000000000..31237d605 --- /dev/null +++ b/tools/netlist/tests/NameTests.cpp @@ -0,0 +1,58 @@ +//------------------------------------------------------------------------------ +//! @file VariableSelectorsTests.cpp +//! @brief Tests for handling of variable selectors. +// +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT +//------------------------------------------------------------------------------ + +#include "NetlistTest.h" + +//===---------------------------------------------------------------------===// +// Tests for name resolution. +//===---------------------------------------------------------------------===// + +TEST_CASE("Unused modules") { + // Test that unused modules are not visited by the netlist builder. + // See Issue #793. + auto tree = SyntaxTree::fromText(R"( +module test (input i1, + input i2, + output o1 + ); + cell_a i_cell_a(.d1(i1), + .d2(i2), + .c(o1)); +endmodule + +module cell_a(input d1, + input d2, + output c); + assign c = d1 + d2; +endmodule + +// unused +module cell_b(input a, + input b, + output z); + assign z = a || b; +endmodule + +// unused +module cell_c(input a, + input b, + output z); + assign z = (!a) && b; +endmodule +)"); + CompilationOptions coptions; + coptions.topModules.emplace("test"sv); + Bag options; + options.set(coptions); + Compilation compilation(options); + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(netlist.numNodes() > 0); +} + diff --git a/tools/netlist/tests/PathTests.cpp b/tools/netlist/tests/PathTests.cpp new file mode 100644 index 000000000..18bb8072d --- /dev/null +++ b/tools/netlist/tests/PathTests.cpp @@ -0,0 +1,492 @@ +//------------------------------------------------------------------------------ +//! @file NetlistTest.cpp +//! @brief Netlist unit tests. +// +// SPDX-FileCopyrightText: Michael Popoloski +// SPDX-License-Identifier: MIT +//------------------------------------------------------------------------------ + +#include "NetlistTest.h" + +//===---------------------------------------------------------------------===// +// Basic tests +//===---------------------------------------------------------------------===// + +TEST_CASE("Empty module") { + // Test the simplest path can be traced through a module. + auto tree = SyntaxTree::fromText(R"( +module empty (input logic i_value, output logic o_value); +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(netlist.numNodes() == 4); + CHECK(netlist.numEdges() == 2); + // Lookup the two ports in the netlist. + auto* inPort = netlist.lookupPort("empty.i_value"); + CHECK(inPort != nullptr); + auto* outPort = netlist.lookupPort("empty.o_value"); + CHECK(outPort != nullptr); +} + +TEST_CASE("Pass through a module") { + // Test the simplest path through a module. + auto tree = SyntaxTree::fromText(R"( +module passthrough (input logic i_value, output logic o_value); + assign o_value = i_value; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(netlist.numNodes() == 6); + CHECK(netlist.numEdges() == 5); + // Lookup the two ports in the netlist. + auto* inPort = netlist.lookupPort("passthrough.i_value"); + CHECK(inPort != nullptr); + auto* outPort = netlist.lookupPort("passthrough.o_value"); + CHECK(outPort != nullptr); + PathFinder pathFinder(netlist); + // Valid i_value -> o_value + CHECK(!pathFinder.find(*inPort, *outPort).empty()); + // Invalid o_value -> i_value + CHECK(pathFinder.find(*outPort, *inPort).empty()); +} + +TEST_CASE("Chain of assignments in a sequence using variables") { + // Test that correct dependencies can be formed from procedural and + // continuous assignments. + auto tree = SyntaxTree::fromText(R"( +module chain_vars (input logic i_value, output logic o_value); + + logic a, b, c, d, e; + + assign a = i_value; + + always_comb begin + b = a; + c = b; + d = c; + end + + assign e = d; + assign o_value = e; + +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(netlist.numNodes() == 21); + CHECK(netlist.numEdges() == 20); + PathFinder pathFinder(netlist); + // Find path i_value -> o_value + auto path = pathFinder.find(*netlist.lookupPort("chain_vars.i_value"), + *netlist.lookupPort("chain_vars.o_value")); + CHECK(path.size() == 12); + CHECK(*path.findVariable("chain_vars.a") == 1); + CHECK(*path.findVariable("chain_vars.b") == 3); + CHECK(*path.findVariable("chain_vars.c") == 5); + CHECK(*path.findVariable("chain_vars.d") == 7); + CHECK(*path.findVariable("chain_vars.e") == 9); +} + +TEST_CASE("Chain of assignments in a sequence using a vector") { + // As above but this time using a packed array. + auto tree = SyntaxTree::fromText(R"( +module chain_array (input logic i_value, output logic o_value); + + logic [4:0] x; + + assign x[0] = i_value; + + always_comb begin + x[1] = x[0]; + x[2] = x[1]; + x[3] = x[2]; + end + + assign x[4] = x[3]; + assign o_value = x[4]; + +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + auto path = pathFinder.find(*netlist.lookupPort("chain_array.i_value"), + *netlist.lookupPort("chain_array.o_value")); + CHECK(*path.findVariable("chain_array.x[0]") == 1); + CHECK(*path.findVariable("chain_array.x[1]") == 3); + CHECK(*path.findVariable("chain_array.x[2]") == 5); + CHECK(*path.findVariable("chain_array.x[3]") == 7); + CHECK(*path.findVariable("chain_array.x[4]") == 9); +} + +TEST_CASE("Passthrough two signals via ranges in a shared vector") { + auto tree = SyntaxTree::fromText(R"( +module passthrough_ranges ( + input logic [1:0] i_value_a, + input logic [1:0] i_value_b, + output logic [1:0] o_value_a, + output logic [1:0] o_value_b +); + + logic [3:0] foo; + + assign foo[1:0] = i_value_a; + assign foo[3:2] = i_value_b; + + assign o_value_a = foo[1:0]; + assign o_value_b = foo[3:2]; + +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + auto* inPortA = netlist.lookupPort("passthrough_ranges.i_value_a"); + auto* inPortB = netlist.lookupPort("passthrough_ranges.i_value_b"); + auto* outPortA = netlist.lookupPort("passthrough_ranges.o_value_a"); + auto* outPortB = netlist.lookupPort("passthrough_ranges.o_value_b"); + PathFinder pathFinder(netlist); + // Valid paths. + CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); + CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); + // Invalid paths. + CHECK(pathFinder.find(*inPortA, *outPortB).empty()); + CHECK(pathFinder.find(*inPortB, *outPortA).empty()); +} + +TEST_CASE("Passthrough two signals via a shared struct") { + auto tree = SyntaxTree::fromText(R"( +module passthrough_member_access ( + input logic i_value_a, + input logic i_value_b, + output logic o_value_a, + output logic o_value_b +); + + struct packed { + logic a; + logic b; + } foo; + + assign foo.a = i_value_a; + assign foo.b = i_value_b; + + assign o_value_a = foo.a; + assign o_value_b = foo.b; + +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + auto* inPortA = netlist.lookupPort("passthrough_member_access.i_value_a"); + auto* inPortB = netlist.lookupPort("passthrough_member_access.i_value_b"); + auto* outPortA = netlist.lookupPort("passthrough_member_access.o_value_a"); + auto* outPortB = netlist.lookupPort("passthrough_member_access.o_value_b"); + PathFinder pathFinder(netlist); + // Valid paths. + CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); + CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); + // Invalid paths. + CHECK(pathFinder.find(*inPortA, *outPortB).empty()); + CHECK(pathFinder.find(*inPortB, *outPortA).empty()); +} + +TEST_CASE("Passthrough two signals via a shared union") { + auto tree = SyntaxTree::fromText(R"( +module passthrough_member_access ( + input logic i_value_a, + input logic i_value_b, + output logic o_value_a, + output logic o_value_b, + output logic o_value_c +); + + union packed { + logic [1:0] a; + logic [1:0] b; + } foo; + + assign foo.a[0] = i_value_a; + assign foo.b[1] = i_value_b; + + assign o_value_a = foo.a[0]; + assign o_value_b = foo.b[1]; + assign o_value_c = foo.b[0]; // Overlapping with a in union. + +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + auto* inPortA = netlist.lookupPort("passthrough_member_access.i_value_a"); + auto* inPortB = netlist.lookupPort("passthrough_member_access.i_value_b"); + auto* outPortA = netlist.lookupPort("passthrough_member_access.o_value_a"); + auto* outPortB = netlist.lookupPort("passthrough_member_access.o_value_b"); + auto* outPortC = netlist.lookupPort("passthrough_member_access.o_value_c"); + PathFinder pathFinder(netlist); + // Valid paths. + CHECK(pathFinder.find(*inPortA, *outPortA).size() == 4); + CHECK(pathFinder.find(*inPortB, *outPortB).size() == 4); + CHECK(pathFinder.find(*inPortA, *outPortC).size() == 4); // Extra path. + // Invalid paths. + CHECK(pathFinder.find(*inPortA, *outPortB).empty()); + CHECK(pathFinder.find(*inPortB, *outPortA).empty()); +} + +//===---------------------------------------------------------------------===// +// Tests for module instance connectivity. +//===---------------------------------------------------------------------===// + +TEST_CASE("Signal passthrough with a nested module") { + // Test that a nested module is connected correctly. + auto tree = SyntaxTree::fromText(R"( +module passthrough(input logic i_value, output logic o_value); + assign o_value = i_value; +endmodule + +module nested_passthrough(input logic i_value, output logic o_value); + passthrough foo( + .i_value(i_value), + .o_value(o_value)); +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + auto path = pathFinder.find(*netlist.lookupPort("nested_passthrough.i_value"), + *netlist.lookupPort("nested_passthrough.o_value")); + CHECK(*path.findVariable("nested_passthrough.foo.i_value") == 1); + CHECK(*path.findVariable("nested_passthrough.foo.o_value") == 2); +} + +TEST_CASE("Signal passthrough with a chain of two nested modules") { + // Test that two nested module are connected correctly. + auto tree = SyntaxTree::fromText(R"( +module passthrough(input logic i_value, output logic o_value); + assign o_value = i_value; +endmodule + +module nested_passthrough(input logic i_value, output logic o_value); + logic value; + passthrough foo_a( + .i_value(i_value), + .o_value(value)); + passthrough foo_b( + .i_value(value), + .o_value(o_value)); +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + auto path = pathFinder.find(*netlist.lookupPort("nested_passthrough.i_value"), + *netlist.lookupPort("nested_passthrough.o_value")); + CHECK(path.size() == 8); + CHECK(*path.findVariable("nested_passthrough.foo_a.i_value") == 1); + CHECK(*path.findVariable("nested_passthrough.foo_a.o_value") == 2); + CHECK(*path.findVariable("nested_passthrough.foo_b.i_value") == 5); + CHECK(*path.findVariable("nested_passthrough.foo_b.o_value") == 6); +} + +//===---------------------------------------------------------------------===// +// Tests for conditional variables in procedural blocks. +//===---------------------------------------------------------------------===// + +TEST_CASE("Mux") { + // Test that the variable in a conditional block is correctly added as a + // dependency on the output variable controlled by that block. + auto tree = SyntaxTree::fromText(R"( +module mux(input a, input b, input sel, output reg f); + always @(*) begin + if (sel == 1'b0) begin + f = a; + end else begin + f = b; + end + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel"), *netlist.lookupPort("mux.f")).empty()); +} + +TEST_CASE("Nested muxing") { + // Test that the variables in multiple nested levels of conditions are + // correctly added as dependencies of the output variable. + auto tree = SyntaxTree::fromText(R"( +module mux(input a, input b, input c, + input sel_a, input sel_b, + output reg f); + always @(*) begin + if (sel_a == 1'b0) begin + if (sel_b == 1'b0) + f = a; + else + f = b; + end else begin + f = c; + end + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + PathFinder pathFinder(netlist); + CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_a"), *netlist.lookupPort("mux.f")).empty()); + CHECK(!pathFinder.find(*netlist.lookupPort("mux.sel_b"), *netlist.lookupPort("mux.f")).empty()); +} + +//===---------------------------------------------------------------------===// +// Tests for loop unrolling +//===---------------------------------------------------------------------===// + +TEST_CASE("Chain of assignments using a single loop") { + // Test that a loop can be unrolled and variable declarations can be split + // out for elements of an array. + auto tree = SyntaxTree::fromText(R"( +module chain_array #(parameter N=4) ( + input logic i_value, + output logic o_value); + + logic pipeline [N-1:0]; + + assign pipeline[0] = i_value; + + always_comb begin + for (int i=1; i o_value, check it passes through each stage. + CHECK(pathFinder + .find(*netlist.lookupPort("chain_array.i_value"), + *netlist.lookupPort("chain_array.o_value")) + .size() == 10); +} + +TEST_CASE("Chain of assignments using a nested loop") { + // Expand the previous test to check the handling of multiple loop variables. + auto tree = SyntaxTree::fromText(R"( +module chain_nested #(parameter N=3) ( + input logic i_value, + output logic o_value); + + logic [(N*N)-1:0] stages; + + //assign stages[0] = i_value; + assign o_value = stages[(N*N)-1]; + + always_comb begin + for (int i=0; i o_value, check it passes through each stage. + auto path = pathFinder.find(*netlist.lookupPort("chain_nested.i_value"), + *netlist.lookupPort("chain_nested.o_value")); + CHECK(*path.findVariable("chain_nested.stages[0]") == 1); + CHECK(*path.findVariable("chain_nested.stages[1]") == 3); + CHECK(*path.findVariable("chain_nested.stages[2]") == 5); + CHECK(*path.findVariable("chain_nested.stages[3]") == 7); + CHECK(*path.findVariable("chain_nested.stages[4]") == 9); + CHECK(*path.findVariable("chain_nested.stages[5]") == 11); + CHECK(*path.findVariable("chain_nested.stages[6]") == 13); + CHECK(*path.findVariable("chain_nested.stages[7]") == 15); + CHECK(*path.findVariable("chain_nested.stages[8]") == 17); +} + +TEST_CASE("Two chains of assignments using a shared 2D array") { + // Check that assignments corresponding to two distinct paths, using the same + // array variable can be handled. + auto tree = SyntaxTree::fromText(R"( +module chain_loop_dual #(parameter N=4)( + input logic i_value_a, + input logic i_value_b, + output logic o_value_a, + output logic o_value_b +); + + parameter A = 0; + parameter B = 1; + + logic pipeline [1:0][N-1:0]; + + assign pipeline[A][0] = i_value_a; + assign pipeline[B][0] = i_value_b; + + always_comb begin + for (int i=1; i Date: Sun, 17 Sep 2023 12:19:12 +0100 Subject: [PATCH 22/31] Add unit tests for struct, union, enum --- tools/netlist/include/Netlist.h | 8 ++- tools/netlist/include/NetlistVisitor.h | 19 ++++-- .../netlist/tests/VariableSelectorsTests.cpp | 65 +++++++++++++++++++ 3 files changed, 84 insertions(+), 8 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 47299b57d..7a2c7a097 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -299,7 +299,13 @@ class NetlistVariableReference : public NetlistNode { } /// Return a string representation of this variable reference. - std::string toString() const { return fmt::format("{}{}", getName(), selectorString()); } + std::string toString() const { + if (selectors.empty()) { + return fmt::format("{}", getName()); + } else { + return fmt::format("{}{}", getName(), selectorString()); + } + } public: /// The expression containing the variable reference. diff --git a/tools/netlist/include/NetlistVisitor.h b/tools/netlist/include/NetlistVisitor.h index f54dc9b43..62b4badea 100644 --- a/tools/netlist/include/NetlistVisitor.h +++ b/tools/netlist/include/NetlistVisitor.h @@ -39,20 +39,20 @@ static std::string getSymbolHierPath(const ast::Symbol& symbol) { return buffer; } -static void connectDeclToVar(Netlist& netlist, NetlistNode& varNode, +static void connectDeclToVar(Netlist& netlist, NetlistNode& declNode, const std::string& hierarchicalPath) { - auto* variableNode = netlist.lookupVariable(hierarchicalPath); - netlist.addEdge(*variableNode, varNode); + auto* varNode = netlist.lookupVariable(hierarchicalPath); + netlist.addEdge(*varNode, declNode); DEBUG_PRINT( - fmt::format("Edge decl {} to ref {}\n", variableNode->getName(), varNode.getName())); + fmt::format("Edge decl {} to ref {}\n", varNode->getName(), declNode.getName())); } static void connectVarToDecl(Netlist& netlist, NetlistNode& varNode, const std::string& hierarchicalPath) { - auto* portNode = netlist.lookupVariable(hierarchicalPath); - netlist.addEdge(varNode, *portNode); + auto* declNode = netlist.lookupVariable(hierarchicalPath); + netlist.addEdge(varNode, *declNode); DEBUG_PRINT( - fmt::format("Edge ref {} to port ref {}\n", varNode.getName(), portNode->getName())); + fmt::format("Edge ref {} to decl {}\n", varNode.getName(), declNode->getName())); } static void connectVarToVar(Netlist& netlist, NetlistNode& sourceVarNode, @@ -72,6 +72,11 @@ class VariableReferenceVisitor : public ast::ASTVisitor Date: Sun, 17 Sep 2023 21:35:46 +0100 Subject: [PATCH 23/31] Fix handing of packed types as scalars --- tools/netlist/include/SplitVariables.h | 43 +++++++------------ .../netlist/tests/VariableSelectorsTests.cpp | 31 +++++++++++-- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index f3a8be34f..e471050f9 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -317,11 +317,15 @@ class AnalyseVariableReference { return {0, getTypeBitWidth(node.symbol.getDeclaredType()->getType()) - 1}; } // Simple vector - if (type.isPredefinedInteger() || type.isScalar()) { + if (type.isPredefinedInteger() || type.isScalar() || + (type.isStruct() && !type.isUnpackedStruct()) || type.isPackedUnion() || + type.isEnum()) { + if (ignoreSelector()) { selectorsIt++; return getBitRangeImpl(type, range); } + if (selectorsIt->get()->isElementSelect()) { return handleScalarElementSelect(type, range); } @@ -336,8 +340,17 @@ class AnalyseVariableReference { default: SLANG_UNREACHABLE; } - } - else { + } else if (selectorsIt->get()->isMemberAccess()) { + if (type.isStruct()) { + return handleStructMemberAccess(type, range); + } else if (type.isPackedUnion()) { + return handleUnionMemberAccess(type, range); + } else if (type.isEnum()) { + return handleEnumMemberAccess(type, range); + } else { + SLANG_ASSERT(0 && "unsupported member selector"); + } + } else { SLANG_ASSERT(0 && "unsupported scalar selector"); } } @@ -366,30 +379,6 @@ class AnalyseVariableReference { SLANG_ASSERT(0 && "unsupported array selector"); } } - // Packed struct - else if (type.isStruct() && !type.isUnpackedStruct()) { - if (selectorsIt->get()->isMemberAccess()) { - return handleStructMemberAccess(type, range); - } else { - SLANG_ASSERT(0 && "unsupported struct selector"); - } - } - // Packed union - else if (type.isPackedUnion()) { - if (selectorsIt->get()->isMemberAccess()) { - return handleUnionMemberAccess(type, range); - } else { - SLANG_ASSERT(0 && "unsupported union selector"); - } - } - // Enum - else if (type.isEnum()) { - if (selectorsIt->get()->isMemberAccess()) { - return handleEnumMemberAccess(type, range); - } else { - SLANG_ASSERT(0 && "unsupported enum selector"); - } - } else { SLANG_ASSERT(0 && "unhandled type in getBitRangeImpl"); } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 1776bbddb..5e9e166c4 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -408,9 +408,9 @@ endmodule TEST_CASE("Enum member") { auto tree = SyntaxTree::fromText(R"( -module m (output int o); - typedef enum logic [7:0] { A, B, C } Foo; - Foo foo; +module m; + typedef enum logic [7:0] { A, B, C } foo_t; + foo_t foo; assign foo = A; endmodule )"); @@ -425,3 +425,28 @@ endmodule // Combine selection of types with arrays, structs, unions and enums. //===---------------------------------------------------------------------===// +TEST_CASE("Struct with packed array members") { + auto tree = SyntaxTree::fromText(R"( +module m; + struct packed { + logic [3:0] a, b; + } foo; + always_comb begin + foo = 0; + foo[1] = 0; + foo[5:1] = 0; + foo.a[2:1] = 0; + foo.b[2:1] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 7)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo[5:1]") == ConstantRange(1, 5)); + CHECK(getBitRange(netlist, "foo.a[2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo.b[2:1]") == ConstantRange(5, 6)); +} From 2e9cf0bf7ce6be3e2661eb96425c5e9c18d3becb Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Mon, 18 Sep 2023 08:46:33 +0100 Subject: [PATCH 24/31] Update TODO.md --- tools/netlist/TODO.md | 61 +------------------------------------------ 1 file changed, 1 insertion(+), 60 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index d33913788..1f925be53 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -1,57 +1,7 @@ To dos ====== -- Bug report: - - module m (input int a); - logic [3:0] [1:0] foo; - always_comb begin - foo[a+:1][1] = 0; - foo[a-:1][1] = 0; - end - endmodule - -/home/jamie/slang/tools/netlist/tests/VariableSelectorsTests.cpp:198: FAILED: -explicitly with message: - source:23:15: warning: cannot refer to element 1 of 'logic[0:0][1:0]' [- - Windex-oob] - foo[a+:1][1] = 0; - ^ - source:24:15: warning: cannot refer to element 1 of 'logic[3:3][1:0]' [- - Windex-oob] - foo[a-:1][1] = 0; - -Similar, baz and bat assignents not valid: - - module m (input int a); - logic foo [1:0]; - logic bar [1:0]; - logic baz [0:0]; - logic bat [1:1]; - always_comb begin - foo = bar; - foo[0] = 0; - foo[1] = 0; - foo[a] = 0; - foo[a+:1] = baz; - foo[a-:1] = bat; - end - endmodule - -- Bug report: - - module m; - logic [7:4] [3:2] foo; - always_comb begin - foo[0] = 0; - end - endmodule - - Assertion 'range.left >= 0 && range.right >= 0' failed - in file /home/jamie/slang/source/ast/symbols/ValueSymbol.cpp, line 436 - function: static std::optional> slang::ast:: - ValueDriver::getBounds(const Expression &, EvalContext &, const Type &) - +- Dumping of a dot file outputs random characters at the end. - Support for more procedural statements, the full list is: InvalidStatement @@ -87,17 +37,8 @@ Similar, baz and bat assignents not valid: RandSequenceStatement ProceduralCheckerStatement -- In SplitVariables, handle: - * Unions, where they cause access cannot to no longer be tracked through a - heirarchy of types. Instead access must be resolved on the bit level (bit blasted). - * Variable positions in element or range selects [x:y]. - Note that in [x+:y] y must be a constant. - Also handle [x-:y]. - - Optimise lookups of nodes in the netlist by adding tables for variable declarations, variable references, ports etc. - -- Dumping of a dot file outputs random characters at the end. - Reporting of variables in the netlist (by type, matching patterns). - Infer sequential elements in the netlist (ie non-blocking assignment and sensitive to a clock edge). From 6142ca2b0e698f093c2ce659baf89aee33dc009e Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Fri, 22 Sep 2023 21:28:13 +0100 Subject: [PATCH 25/31] Add union test --- .../netlist/tests/VariableSelectorsTests.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 5e9e166c4..b304424f8 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -450,3 +450,31 @@ endmodule CHECK(getBitRange(netlist, "foo.a[2:1]") == ConstantRange(1, 2)); CHECK(getBitRange(netlist, "foo.b[2:1]") == ConstantRange(5, 6)); } + +TEST_CASE("Struct with packed union members") { + auto tree = SyntaxTree::fromText(R"( +module m; + struct packed { + union packed { + logic [3:0] a, b; + } u; + } foo; + always_comb begin + foo = 0; + foo[1] = 0; + foo.u[2:1] = 0; + foo.u.a[2:1] = 0; + foo.u.b[2:1] = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo.u[2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo.u.a[2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo.u.b[2:1]") == ConstantRange(1, 2)); +} From f8446d5db0ee881a13657a4d1a0ee21c62c98395 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sat, 23 Sep 2023 21:59:50 +0100 Subject: [PATCH 26/31] More unit tests and tidying up --- tools/netlist/TODO.md | 1 + tools/netlist/include/SplitVariables.h | 13 +- tools/netlist/tests/PathTests.cpp | 35 ++++++ .../netlist/tests/VariableSelectorsTests.cpp | 117 ++++++++++++++++-- 4 files changed, 147 insertions(+), 19 deletions(-) diff --git a/tools/netlist/TODO.md b/tools/netlist/TODO.md index 1f925be53..03cf4eeda 100644 --- a/tools/netlist/TODO.md +++ b/tools/netlist/TODO.md @@ -1,6 +1,7 @@ To dos ====== +- Support descending ranges in split variable type handling, eg [0:3]. - Dumping of a dot file outputs random characters at the end. - Support for more procedural statements, the full list is: diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index e471050f9..d36e8e220 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -261,7 +261,7 @@ class AnalyseVariableReference { if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; const auto& fieldType = getScopeFieldType(packedStruct, memberAccessSelector.name); - return getBitRangeImpl(fieldType, fieldRange); + return getBitRangeImpl(fieldType, newRange); } else { return newRange; } @@ -278,7 +278,7 @@ class AnalyseVariableReference { if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; const auto& fieldType = getScopeFieldType(packedUnion, memberAccessSelector.name); - return getBitRangeImpl(fieldType, fieldRange); + return getBitRangeImpl(fieldType, newRange); } else { return newRange; } @@ -292,13 +292,8 @@ class AnalyseVariableReference { ConstantRange newRange = {range.lower() + fieldRange.lower(), range.lower() + fieldRange.upper()}; SLANG_ASSERT(range.contains(newRange)); - if (std::next(selectorsIt) != node.selectors.end()) { - selectorsIt++; - const auto& fieldType = getScopeFieldType(enumeration, memberAccessSelector.name); - return getBitRangeImpl(fieldType, fieldRange); - } else { - return newRange; - } + SLANG_ASSERT(std::next(selectorsIt) == node.selectors.end()); + return newRange; } // Multiple range selectors have only the effect of the last one. diff --git a/tools/netlist/tests/PathTests.cpp b/tools/netlist/tests/PathTests.cpp index 18bb8072d..d96729095 100644 --- a/tools/netlist/tests/PathTests.cpp +++ b/tools/netlist/tests/PathTests.cpp @@ -490,3 +490,38 @@ endmodule CHECK(pathFinder.find(*inPortB, *outPortA).empty()); } +//===---------------------------------------------------------------------===// +// Test case for #792 (bus expression in ports) +//===---------------------------------------------------------------------===// + +TEST_CASE("Test case for #792 (bus expression in ports)") { + auto tree = SyntaxTree::fromText(R"( +module test (input [1:0] in_i, + output [1:0] out_o); + + wire [1:0] in_s; + + assign in_s = in_i; + + nop i_nop(.in_i(in_s[1:0]), // ok: in_s, in_i, {in_i[1], in_i[0]} + .out_o(out_o)); +endmodule + +module nop (input [1:0] in_i, + output [1:0] out_o); + + // individual bits access; ok: out_o = in_i; + assign out_o[0] = in_i[0]; + assign out_o[1] = in_i[1]; +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + auto* inPort = netlist.lookupPort("test.in_i"); + auto* outPort = netlist.lookupPort("test.out_o"); + PathFinder pathFinder(netlist); + // Valid paths. + CHECK(!pathFinder.find(*inPort, *outPort).empty()); +} diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index b304424f8..6d4540e67 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -193,8 +193,6 @@ module m (input int a); foo[a][a] = 0; foo[a+:1] = 0; foo[a-:1] = 0; - //foo[a+:1][1] = 0; - //foo[a-:1][1] = 0; foo[1][a] = 0; foo[1][a+:1] = 0; foo[1][a-:1] = 0; @@ -224,8 +222,6 @@ endmodule CHECK(getBitRange(netlist, "foo[a][a]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[a-:1]") == ConstantRange(0, 7)); - //CHECK(getBitRange(netlist, "foo[a+:1][1]") == ConstantRange(0, 7)); - //CHECK(getBitRange(netlist, "foo[a-:1][1]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[1][a]") == ConstantRange(2, 3)); CHECK(getBitRange(netlist, "foo[1][a+:1]") == ConstantRange(2, 3)); CHECK(getBitRange(netlist, "foo[1][a-:1]") == ConstantRange(2, 3)); @@ -321,8 +317,6 @@ module m (input int a); foo[a][a] = 0; foo[a+:1] = '{'{0, 0}}; foo[a-:2] = '{'{0, 0}, '{0, 0}}; - //foo[a+:1][1] = 0; - //foo[a-:1][1] = 0; foo[1][a] = 0; foo[1][a+:1] = '{0}; foo[1][a-:2] = '{0, 0}; @@ -345,8 +339,6 @@ endmodule CHECK(getBitRange(netlist, "foo[a][a]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[a+:1]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[a-:2]") == ConstantRange(0, 7)); - //CHECK(getBitRange(netlist, "foo[a+:1][1]") == ConstantRange(0, 7)); - //CHECK(getBitRange(netlist, "foo[a-:1][1]") == ConstantRange(0, 7)); CHECK(getBitRange(netlist, "foo[1][a]") == ConstantRange(2, 3)); CHECK(getBitRange(netlist, "foo[1][a+:1]") == ConstantRange(2, 3)); CHECK(getBitRange(netlist, "foo[1][a-:2]") == ConstantRange(2, 3)); @@ -426,6 +418,7 @@ endmodule //===---------------------------------------------------------------------===// TEST_CASE("Struct with packed array members") { + // Test recursion from packed struct. auto tree = SyntaxTree::fromText(R"( module m; struct packed { @@ -451,20 +444,25 @@ endmodule CHECK(getBitRange(netlist, "foo.b[2:1]") == ConstantRange(5, 6)); } -TEST_CASE("Struct with packed union members") { +TEST_CASE("Packed struct with packed union and enum members") { + // Test recursion from packed struct. auto tree = SyntaxTree::fromText(R"( module m; + typedef enum int { A, B, C } enum_t; struct packed { union packed { logic [3:0] a, b; } u; + enum_t c; } foo; always_comb begin foo = 0; foo[1] = 0; + foo.u = 0; foo.u[2:1] = 0; foo.u.a[2:1] = 0; foo.u.b[2:1] = 0; + foo.c = A; end endmodule )"); @@ -472,9 +470,108 @@ endmodule compilation.addSyntaxTree(tree); NO_COMPILATION_ERRORS; auto netlist = createNetlist(compilation); - CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 35)); CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(1, 1)); + CHECK(getBitRange(netlist, "foo.u") == ConstantRange(0, 3)); CHECK(getBitRange(netlist, "foo.u[2:1]") == ConstantRange(1, 2)); CHECK(getBitRange(netlist, "foo.u.a[2:1]") == ConstantRange(1, 2)); CHECK(getBitRange(netlist, "foo.u.b[2:1]") == ConstantRange(1, 2)); + CHECK(getBitRange(netlist, "foo.c") == ConstantRange(4, 35)); +} + +TEST_CASE("Packed arrays of structs etc") { + // Test recursion from packed packed array, packed struct, packed union. + auto tree = SyntaxTree::fromText(R"( +module m; + typedef enum int { A, B, C } enum_t; + typedef struct packed { + union packed { + logic [3:0] a, b; + } u; + logic [1:0] c; + enum_t d; + } foo_t; + foo_t [3:0] [1:0] foo; + always_comb begin + foo = 0; + foo[0] = 0; + foo[1] = 0; + foo[0][0] = 0; + foo[0][1] = 0; + foo[0][0].u.a = 0; + foo[0][1].u.a = 0; + foo[0][0].u.b = 0; + foo[0][1].u.b = 0; + foo[0][0].c = 0; + foo[0][1].c = 0; + foo[0][0].d = A; + foo[0][1].d = A; + foo[3][1] = 0; + foo[3][1].u.a = 0; + foo[3][1].u.b = 0; + foo[3][1].c = 0; + foo[3][1].d = A; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 303)); + CHECK(getBitRange(netlist, "foo[0]") == ConstantRange(0, 75)); + CHECK(getBitRange(netlist, "foo[1]") == ConstantRange(76, 151)); + CHECK(getBitRange(netlist, "foo[0][0]") == ConstantRange(0, 37)); + CHECK(getBitRange(netlist, "foo[0][1]") == ConstantRange(38, 75)); + CHECK(getBitRange(netlist, "foo[0][0].u.a") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[0][1].u.a") == ConstantRange(38, 41)); + CHECK(getBitRange(netlist, "foo[0][0].u.b") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[0][1].u.b") == ConstantRange(38, 41)); + CHECK(getBitRange(netlist, "foo[0][0].c") == ConstantRange(4, 5)); + CHECK(getBitRange(netlist, "foo[0][1].c") == ConstantRange(42, 43)); + CHECK(getBitRange(netlist, "foo[0][0].d") == ConstantRange(6, 37)); + CHECK(getBitRange(netlist, "foo[0][1].d") == ConstantRange(44, 75)); + CHECK(getBitRange(netlist, "foo[3][1]") == ConstantRange(266, 303)); + CHECK(getBitRange(netlist, "foo[3][1].u.a") == ConstantRange(266, 269)); + CHECK(getBitRange(netlist, "foo[3][1].u.b") == ConstantRange(266, 269)); + CHECK(getBitRange(netlist, "foo[3][1].c") == ConstantRange(270, 271)); + CHECK(getBitRange(netlist, "foo[3][1].d") == ConstantRange(272, 303)); +} + +TEST_CASE("Union with packed struct members") { + // Test recursion from packed union, packed struct. + auto tree = SyntaxTree::fromText(R"( +module m; + typedef enum int { A, B, C } enum_t; + union packed { + struct packed { + logic [3:0] a, b; + } x, y; + } [3:0] foo; + always_comb begin + foo = 0; + foo[0].x.a = 0; + foo[0].x.b = 0; + foo[0].y.a = 0; + foo[0].y.b = 0; + foo[3].x.a = 0; + foo[3].x.b = 0; + foo[3].y.a = 0; + foo[3].y.b = 0; + end +endmodule +)"); + Compilation compilation; + compilation.addSyntaxTree(tree); + NO_COMPILATION_ERRORS; + auto netlist = createNetlist(compilation); + CHECK(getBitRange(netlist, "foo") == ConstantRange(0, 31)); + CHECK(getBitRange(netlist, "foo[0].x.a") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[0].x.b") == ConstantRange(4, 7)); + CHECK(getBitRange(netlist, "foo[0].y.a") == ConstantRange(0, 3)); + CHECK(getBitRange(netlist, "foo[0].y.b") == ConstantRange(4, 7)); + CHECK(getBitRange(netlist, "foo[3].x.a") == ConstantRange(24, 27)); + CHECK(getBitRange(netlist, "foo[3].x.b") == ConstantRange(28, 31)); + CHECK(getBitRange(netlist, "foo[3].y.a") == ConstantRange(24, 27)); + CHECK(getBitRange(netlist, "foo[3].y.b") == ConstantRange(28, 31)); } From efc9937e8698720ef05cef6d6c094ace0ac61fc0 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 23 Sep 2023 21:06:27 +0000 Subject: [PATCH 27/31] style: pre-commit fixes --- tools/netlist/include/Netlist.h | 74 ++++--- tools/netlist/include/NetlistVisitor.h | 12 +- tools/netlist/include/SplitVariables.h | 185 ++++++++++-------- tools/netlist/tests/CMakeLists.txt | 8 +- tools/netlist/tests/NameTests.cpp | 1 - tools/netlist/tests/NetlistTest.h | 4 +- .../netlist/tests/VariableSelectorsTests.cpp | 11 +- 7 files changed, 153 insertions(+), 142 deletions(-) diff --git a/tools/netlist/include/Netlist.h b/tools/netlist/include/Netlist.h index 7a2c7a097..202df52a1 100644 --- a/tools/netlist/include/Netlist.h +++ b/tools/netlist/include/Netlist.h @@ -67,7 +67,7 @@ struct VariableSelectorBase { /// A variable selector representing an element selector. struct VariableElementSelect : public VariableSelectorBase { - const ast::Expression &expr; + const ast::Expression& expr; ConstantValue index; VariableElementSelect(ast::Expression const& expr, ConstantValue index) : @@ -78,9 +78,7 @@ struct VariableElementSelect : public VariableSelectorBase { return otherKind == VariableSelectorKind::ElementSelect; } - bool indexIsConstant() const { - return !index.bad(); - } + bool indexIsConstant() const { return !index.bad(); } int32_t getIndexInt() const { auto intValue = index.integer().as(); @@ -89,17 +87,18 @@ struct VariableElementSelect : public VariableSelectorBase { } std::string toString() const override { - if (indexIsConstant()) { - return fmt::format("[{}]", index.toString()); - } else { - return fmt::format("[{}]", expr.syntax->toString()); - } + if (indexIsConstant()) { + return fmt::format("[{}]", index.toString()); + } + else { + return fmt::format("[{}]", expr.syntax->toString()); + } } }; /// A variable selector representing a range selector. struct VariableRangeSelect : public VariableSelectorBase { - const ast::RangeSelectExpression &expr; + const ast::RangeSelectExpression& expr; ConstantValue leftIndex, rightIndex; VariableRangeSelect(ast::RangeSelectExpression const& expr, ConstantValue leftIndex, @@ -111,13 +110,9 @@ struct VariableRangeSelect : public VariableSelectorBase { return otherKind == VariableSelectorKind::RangeSelect; } - bool leftIndexIsConstant() const { - return !leftIndex.bad(); - } + bool leftIndexIsConstant() const { return !leftIndex.bad(); } - bool rightIndexIsConstant() const { - return !rightIndex.bad(); - } + bool rightIndexIsConstant() const { return !rightIndex.bad(); } int32_t getLeftIndexInt() const { auto intValue = leftIndex.integer().as(); @@ -134,25 +129,27 @@ struct VariableRangeSelect : public VariableSelectorBase { std::string toString() const override { std::string left; if (leftIndexIsConstant()) { - left = leftIndex.toString(); - } else { - left = expr.left().syntax->toString(); + left = leftIndex.toString(); + } + else { + left = expr.left().syntax->toString(); } std::string right; if (rightIndexIsConstant()) { - right = rightIndex.toString(); - } else { - right = expr.right().syntax->toString(); + right = rightIndex.toString(); + } + else { + right = expr.right().syntax->toString(); } switch (expr.getSelectionKind()) { - case ast::RangeSelectionKind::Simple: - return fmt::format("[{}:{}]", left, right); - case ast::RangeSelectionKind::IndexedUp: - return fmt::format("[{}+:{}]", left, right); - case ast::RangeSelectionKind::IndexedDown: - return fmt::format("[{}-:{}]", left, right); - default: - SLANG_UNREACHABLE; + case ast::RangeSelectionKind::Simple: + return fmt::format("[{}:{}]", left, right); + case ast::RangeSelectionKind::IndexedUp: + return fmt::format("[{}+:{}]", left, right); + case ast::RangeSelectionKind::IndexedDown: + return fmt::format("[{}-:{}]", left, right); + default: + SLANG_UNREACHABLE; } } }; @@ -271,12 +268,12 @@ class NetlistVariableReference : public NetlistNode { NetlistNode(NodeKind::VariableReference, symbol), expression(expr), leftOperand(leftOperand) {} - void addElementSelect(ast::ElementSelectExpression const &expr, const ConstantValue& index) { + void addElementSelect(ast::ElementSelectExpression const& expr, const ConstantValue& index) { selectors.emplace_back(std::make_unique(expr.selector(), index)); } - void addRangeSelect(ast::RangeSelectExpression const& expr, - const ConstantValue& leftIndex, const ConstantValue& rightIndex) { + void addRangeSelect(ast::RangeSelectExpression const& expr, const ConstantValue& leftIndex, + const ConstantValue& rightIndex) { selectors.emplace_back(std::make_unique(expr, leftIndex, rightIndex)); } @@ -300,11 +297,12 @@ class NetlistVariableReference : public NetlistNode { /// Return a string representation of this variable reference. std::string toString() const { - if (selectors.empty()) { - return fmt::format("{}", getName()); - } else { - return fmt::format("{}{}", getName(), selectorString()); - } + if (selectors.empty()) { + return fmt::format("{}", getName()); + } + else { + return fmt::format("{}{}", getName(), selectorString()); + } } public: diff --git a/tools/netlist/include/NetlistVisitor.h b/tools/netlist/include/NetlistVisitor.h index 62b4badea..1e9b142d6 100644 --- a/tools/netlist/include/NetlistVisitor.h +++ b/tools/netlist/include/NetlistVisitor.h @@ -43,16 +43,14 @@ static void connectDeclToVar(Netlist& netlist, NetlistNode& declNode, const std::string& hierarchicalPath) { auto* varNode = netlist.lookupVariable(hierarchicalPath); netlist.addEdge(*varNode, declNode); - DEBUG_PRINT( - fmt::format("Edge decl {} to ref {}\n", varNode->getName(), declNode.getName())); + DEBUG_PRINT(fmt::format("Edge decl {} to ref {}\n", varNode->getName(), declNode.getName())); } static void connectVarToDecl(Netlist& netlist, NetlistNode& varNode, const std::string& hierarchicalPath) { auto* declNode = netlist.lookupVariable(hierarchicalPath); netlist.addEdge(varNode, *declNode); - DEBUG_PRINT( - fmt::format("Edge ref {} to decl {}\n", varNode.getName(), declNode->getName())); + DEBUG_PRINT(fmt::format("Edge ref {} to decl {}\n", varNode.getName(), declNode->getName())); } static void connectVarToVar(Netlist& netlist, NetlistNode& sourceVarNode, @@ -73,9 +71,9 @@ class VariableReferenceVisitor : public ast::ASTVisitor getTypeBitWidthImpl(slang::ast::Type const& type) { size_t fixedSize = type.getBitWidth(); @@ -55,24 +54,24 @@ class AnalyseVariableReference { /// Return the bit width of a slang type, treating unpacked arrays as /// as if they were packed. - int32_t getTypeBitWidth(slang::ast::Type const &type) { - auto [multiplierElem, fixedSizeElem] = getTypeBitWidthImpl(type); - return (int32_t) (multiplierElem * fixedSizeElem); + int32_t getTypeBitWidth(slang::ast::Type const& type) { + auto [multiplierElem, fixedSizeElem] = getTypeBitWidthImpl(type); + return (int32_t)(multiplierElem * fixedSizeElem); } /// Given a scope, return the type of the named field. slang::ast::Type const& getScopeFieldType(const slang::ast::Scope& scope, - const std::string_view name) const { + const std::string_view name) const { auto* symbol = scope.find(name); SLANG_ASSERT(symbol != nullptr); return symbol->getDeclaredType()->getType(); } /// Given a packed struct type, return the bit position of the named field. - ConstantRange getStructFieldRange(const slang::ast::PackedStructType &packedStruct, + ConstantRange getStructFieldRange(const slang::ast::PackedStructType& packedStruct, const std::string_view fieldName) const { int32_t offset = 0; - for (auto &member : packedStruct.members()) { + for (auto& member : packedStruct.members()) { int32_t fieldWidth = member.getDeclaredType()->getType().getBitWidth(); if (member.name == fieldName) { return {offset, offset + fieldWidth - 1}; @@ -83,7 +82,7 @@ class AnalyseVariableReference { } /// Given a packed union type, return the bit position of the named field. - ConstantRange getUnionFieldRange(const slang::ast::PackedUnionType &packedUnion, + ConstantRange getUnionFieldRange(const slang::ast::PackedUnionType& packedUnion, const std::string_view fieldName) const { auto* symbol = packedUnion.find(fieldName); SLANG_ASSERT(symbol != nullptr); @@ -98,7 +97,7 @@ class AnalyseVariableReference { } /// Given an array type, return the range from which the array is indexed. - ConstantRange getArrayRange(const slang::ast::Type &type) { + ConstantRange getArrayRange(const slang::ast::Type& type) { if (type.kind == slang::ast::SymbolKind::PackedArrayType) { auto& arrayType = type.as(); return arrayType.range; @@ -108,17 +107,17 @@ class AnalyseVariableReference { return arrayType.range; } else { - SLANG_ASSERT(0 && "unexpected array type"); + SLANG_ASSERT(0 && "unexpected array type"); } } ConstantRange handleScalarElementSelect(const slang::ast::Type& type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); if (!elementSelector.indexIsConstant()) { - // If the selector is not a constant, then return the whole scalar as - // the range. No further selectors expected. - SLANG_ASSERT(std::next(selectorsIt) == node.selectors.end()); - return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; + // If the selector is not a constant, then return the whole scalar as + // the range. No further selectors expected. + SLANG_ASSERT(std::next(selectorsIt) == node.selectors.end()); + return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; } // Create a new range. int32_t index = range.lower() + elementSelector.getIndexInt(); @@ -126,7 +125,7 @@ class AnalyseVariableReference { return {index, index}; } - ConstantRange handleScalarRangeSelect(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleScalarRangeSelect(const slang::ast::Type& type, ConstantRange range) { const auto& rangeSelector = selectorsIt->get()->as(); int32_t rightIndex = rangeSelector.getRightIndexInt(); int32_t leftIndex = rangeSelector.getLeftIndexInt(); @@ -134,24 +133,25 @@ class AnalyseVariableReference { SLANG_ASSERT(rangeSelector.leftIndexIsConstant()); SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); // Create a new range. - ConstantRange newRange = {range.lower() + rightIndex, - range.lower() + leftIndex}; + ConstantRange newRange = {range.lower() + rightIndex, range.lower() + leftIndex}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); - } else { + } + else { return newRange; } } - ConstantRange handleScalarRangeSelectIncr(const slang::ast::Type &type, ConstantRange range, bool isUp) { + ConstantRange handleScalarRangeSelectIncr(const slang::ast::Type& type, ConstantRange range, + bool isUp) { const auto& rangeSelector = selectorsIt->get()->as(); if (!rangeSelector.leftIndexIsConstant()) { - // If the selector base is not constant, then return the whole scalar - // as the range and halt analysis of any further selectors. - selectorsIt = node.selectors.end(); - return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; + // If the selector base is not constant, then return the whole scalar + // as the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), range.lower() + (int32_t)type.getBitWidth() - 1}; } // Right index must be constant. SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); @@ -159,24 +159,24 @@ class AnalyseVariableReference { int32_t leftIndex = rangeSelector.getLeftIndexInt(); // Create a new range. auto rangeEnd = isUp ? rightIndex + leftIndex : rightIndex - leftIndex; - ConstantRange newRange = {range.lower() + rightIndex, - range.lower() + rangeEnd}; + ConstantRange newRange = {range.lower() + rightIndex, range.lower() + rangeEnd}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); - } else { + } + else { return newRange; } } - ConstantRange handleArrayElementSelect(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleArrayElementSelect(const slang::ast::Type& type, ConstantRange range) { const auto& elementSelector = selectorsIt->get()->as(); if (!elementSelector.indexIsConstant()) { - // If the selector is not a constant, then return the whole scalar as - // the range and halt analysis of any further selectors. - selectorsIt = node.selectors.end(); - return {range.lower(), range.lower() + getTypeBitWidth(type) - 1}; + // If the selector is not a constant, then return the whole scalar as + // the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), range.lower() + getTypeBitWidth(type) - 1}; } int32_t index = elementSelector.getIndexInt(); auto arrayRange = getArrayRange(type); @@ -186,17 +186,19 @@ class AnalyseVariableReference { // Create a new range. auto* elementType = type.getArrayElementType(); ConstantRange newRange = {range.lower() + (index * getTypeBitWidth(*elementType)), - range.lower() + ((index + 1) * getTypeBitWidth(*elementType)) - 1}; + range.lower() + ((index + 1) * getTypeBitWidth(*elementType)) - + 1}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(*elementType, newRange); - } else { + } + else { return newRange; } } - ConstantRange handleArrayRangeSelect(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleArrayRangeSelect(const slang::ast::Type& type, ConstantRange range) { const auto& rangeSelector = selectorsIt->get()->as(); int32_t leftIndex = rangeSelector.getLeftIndexInt(); int32_t rightIndex = rangeSelector.getRightIndexInt(); @@ -210,26 +212,30 @@ class AnalyseVariableReference { // Create a new range. auto* elementType = type.getArrayElementType(); ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), - range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; + range.lower() + + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); - } else { + } + else { return newRange; } } - ConstantRange handleArrayRangeSelectIncr(const slang::ast::Type &type, ConstantRange range, bool isUp) { + ConstantRange handleArrayRangeSelectIncr(const slang::ast::Type& type, ConstantRange range, + bool isUp) { const auto& rangeSelector = selectorsIt->get()->as(); auto* elementType = type.getArrayElementType(); auto arrayRange = getArrayRange(type); if (!rangeSelector.leftIndexIsConstant()) { - // If the selector base is not constant, then return the whole array - // as the range and halt analysis of any further selectors. - selectorsIt = node.selectors.end(); - return {range.lower(), - range.lower() + (getTypeBitWidth(*elementType) * (int32_t)arrayRange.width()) - 1}; + // If the selector base is not constant, then return the whole array + // as the range and halt analysis of any further selectors. + selectorsIt = node.selectors.end(); + return {range.lower(), + range.lower() + (getTypeBitWidth(*elementType) * (int32_t)arrayRange.width()) - + 1}; } // Right index must be constant. SLANG_ASSERT(rangeSelector.rightIndexIsConstant()); @@ -240,17 +246,19 @@ class AnalyseVariableReference { rightIndex -= arrayRange.lower(); // Create a new range. ConstantRange newRange = {range.lower() + (rightIndex * getTypeBitWidth(*elementType)), - range.lower() + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; + range.lower() + + ((leftIndex + 1) * getTypeBitWidth(*elementType)) - 1}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { selectorsIt++; return getBitRangeImpl(type, newRange); - } else { + } + else { return newRange; } } - ConstantRange handleStructMemberAccess(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleStructMemberAccess(const slang::ast::Type& type, ConstantRange range) { const auto& memberAccessSelector = selectorsIt->get()->as(); const auto& packedStruct = type.getCanonicalType().as(); auto fieldRange = getStructFieldRange(packedStruct, memberAccessSelector.name); @@ -259,15 +267,16 @@ class AnalyseVariableReference { range.lower() + fieldRange.upper()}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { - selectorsIt++; - const auto& fieldType = getScopeFieldType(packedStruct, memberAccessSelector.name); - return getBitRangeImpl(fieldType, newRange); - } else { - return newRange; + selectorsIt++; + const auto& fieldType = getScopeFieldType(packedStruct, memberAccessSelector.name); + return getBitRangeImpl(fieldType, newRange); + } + else { + return newRange; } } - ConstantRange handleUnionMemberAccess(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleUnionMemberAccess(const slang::ast::Type& type, ConstantRange range) { const auto& memberAccessSelector = selectorsIt->get()->as(); const auto& packedUnion = type.getCanonicalType().as(); auto fieldRange = getUnionFieldRange(packedUnion, memberAccessSelector.name); @@ -276,15 +285,16 @@ class AnalyseVariableReference { range.lower() + fieldRange.upper()}; SLANG_ASSERT(range.contains(newRange)); if (std::next(selectorsIt) != node.selectors.end()) { - selectorsIt++; - const auto& fieldType = getScopeFieldType(packedUnion, memberAccessSelector.name); - return getBitRangeImpl(fieldType, newRange); - } else { - return newRange; + selectorsIt++; + const auto& fieldType = getScopeFieldType(packedUnion, memberAccessSelector.name); + return getBitRangeImpl(fieldType, newRange); + } + else { + return newRange; } } - ConstantRange handleEnumMemberAccess(const slang::ast::Type &type, ConstantRange range) { + ConstantRange handleEnumMemberAccess(const slang::ast::Type& type, ConstantRange range) { const auto& memberAccessSelector = selectorsIt->get()->as(); const auto& enumeration = type.getCanonicalType().as(); auto fieldRange = getEnumRange(enumeration); @@ -306,7 +316,7 @@ class AnalyseVariableReference { /// Given a variable reference with zero or more selectors, determine the /// bit range that is accessed. - ConstantRange getBitRangeImpl(const slang::ast::Type &type, ConstantRange range) { + ConstantRange getBitRangeImpl(const slang::ast::Type& type, ConstantRange range) { // No selectors if (node.selectors.empty()) { return {0, getTypeBitWidth(node.symbol.getDeclaredType()->getType()) - 1}; @@ -326,26 +336,31 @@ class AnalyseVariableReference { } else if (selectorsIt->get()->isRangeSelect()) { switch (selectorsIt->get()->as().expr.getSelectionKind()) { - case ast::RangeSelectionKind::Simple: - return handleScalarRangeSelect(type, range); - case ast::RangeSelectionKind::IndexedUp: - return handleScalarRangeSelectIncr(type, range, true); - case ast::RangeSelectionKind::IndexedDown: - return handleScalarRangeSelectIncr(type, range, false); - default: - SLANG_UNREACHABLE; + case ast::RangeSelectionKind::Simple: + return handleScalarRangeSelect(type, range); + case ast::RangeSelectionKind::IndexedUp: + return handleScalarRangeSelectIncr(type, range, true); + case ast::RangeSelectionKind::IndexedDown: + return handleScalarRangeSelectIncr(type, range, false); + default: + SLANG_UNREACHABLE; } - } else if (selectorsIt->get()->isMemberAccess()) { + } + else if (selectorsIt->get()->isMemberAccess()) { if (type.isStruct()) { return handleStructMemberAccess(type, range); - } else if (type.isPackedUnion()) { + } + else if (type.isPackedUnion()) { return handleUnionMemberAccess(type, range); - } else if (type.isEnum()) { + } + else if (type.isEnum()) { return handleEnumMemberAccess(type, range); - } else { + } + else { SLANG_ASSERT(0 && "unsupported member selector"); } - } else { + } + else { SLANG_ASSERT(0 && "unsupported scalar selector"); } } @@ -360,14 +375,14 @@ class AnalyseVariableReference { } else if (selectorsIt->get()->isRangeSelect()) { switch (selectorsIt->get()->as().expr.getSelectionKind()) { - case ast::RangeSelectionKind::Simple: - return handleArrayRangeSelect(type, range); - case ast::RangeSelectionKind::IndexedUp: - return handleArrayRangeSelectIncr(type, range, true); - case ast::RangeSelectionKind::IndexedDown: - return handleArrayRangeSelectIncr(type, range, false); - default: - SLANG_UNREACHABLE; + case ast::RangeSelectionKind::Simple: + return handleArrayRangeSelect(type, range); + case ast::RangeSelectionKind::IndexedUp: + return handleArrayRangeSelectIncr(type, range, true); + case ast::RangeSelectionKind::IndexedDown: + return handleArrayRangeSelectIncr(type, range, false); + default: + SLANG_UNREACHABLE; } } else { @@ -394,13 +409,13 @@ class SplitVariables { SplitVariables(Netlist& netlist) : netlist(netlist) { split(); } private: - /// Return true if the selection made by the target node intersects with the /// selection made by the source node. bool isIntersectingSelection(NetlistVariableReference& sourceNode, NetlistVariableReference& targetNode) const { - return AnalyseVariableReference(sourceNode).getBitRange().overlaps( - AnalyseVariableReference(targetNode).getBitRange()); + return AnalyseVariableReference(sourceNode) + .getBitRange() + .overlaps(AnalyseVariableReference(targetNode).getBitRange()); } void split() { diff --git a/tools/netlist/tests/CMakeLists.txt b/tools/netlist/tests/CMakeLists.txt index f4ecb2cdf..0afc4a9e3 100644 --- a/tools/netlist/tests/CMakeLists.txt +++ b/tools/netlist/tests/CMakeLists.txt @@ -5,9 +5,13 @@ add_executable( netlist_unittests - ../../../tests/unittests/main.cpp ../../../tests/unittests/Test.cpp + ../../../tests/unittests/main.cpp + ../../../tests/unittests/Test.cpp ../source/Netlist.cpp - DepthFirstSearchTests.cpp DirectedGraphTests.cpp NameTests.cpp PathTests.cpp + DepthFirstSearchTests.cpp + DirectedGraphTests.cpp + NameTests.cpp + PathTests.cpp VariableSelectorsTests.cpp) target_link_libraries( diff --git a/tools/netlist/tests/NameTests.cpp b/tools/netlist/tests/NameTests.cpp index 31237d605..841ff83db 100644 --- a/tools/netlist/tests/NameTests.cpp +++ b/tools/netlist/tests/NameTests.cpp @@ -55,4 +55,3 @@ endmodule auto netlist = createNetlist(compilation); CHECK(netlist.numNodes() > 0); } - diff --git a/tools/netlist/tests/NetlistTest.h b/tools/netlist/tests/NetlistTest.h index 33d94fc07..f92b6a88f 100644 --- a/tools/netlist/tests/NetlistTest.h +++ b/tools/netlist/tests/NetlistTest.h @@ -6,7 +6,6 @@ #include "PathFinder.h" #include "SplitVariables.h" #include "Test.h" - #include using namespace netlist; @@ -19,10 +18,9 @@ inline Netlist createNetlist(Compilation& compilation) { return netlist; } -inline bool pathExists(Netlist &netlist, const std::string& startName, const std::string& endName) { +inline bool pathExists(Netlist& netlist, const std::string& startName, const std::string& endName) { PathFinder pathFinder(netlist); auto* startNode = netlist.lookupPort(startName); auto* endNode = netlist.lookupPort(endName); return !pathFinder.find(*startNode, *endNode).empty(); } - diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 6d4540e67..7aa1c6ae2 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -10,13 +10,12 @@ #include "SplitVariables.h" #include - /// Helper method to extract a variable reference from a netlist and return the /// bit range associated with it. -ConstantRange getBitRange(Netlist &netlist, std::string_view variableSyntax) { +ConstantRange getBitRange(Netlist& netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); if (node == nullptr) { - throw std::runtime_error(fmt::format("Could not find node {}", variableSyntax)); + throw std::runtime_error(fmt::format("Could not find node {}", variableSyntax)); } return AnalyseVariableReference::create(*node).getBitRange(); } @@ -418,7 +417,7 @@ endmodule //===---------------------------------------------------------------------===// TEST_CASE("Struct with packed array members") { - // Test recursion from packed struct. + // Test recursion from packed struct. auto tree = SyntaxTree::fromText(R"( module m; struct packed { @@ -445,7 +444,7 @@ endmodule } TEST_CASE("Packed struct with packed union and enum members") { - // Test recursion from packed struct. + // Test recursion from packed struct. auto tree = SyntaxTree::fromText(R"( module m; typedef enum int { A, B, C } enum_t; @@ -480,7 +479,7 @@ endmodule } TEST_CASE("Packed arrays of structs etc") { - // Test recursion from packed packed array, packed struct, packed union. + // Test recursion from packed packed array, packed struct, packed union. auto tree = SyntaxTree::fromText(R"( module m; typedef enum int { A, B, C } enum_t; From 6b7d194a7e8447f31e7f4f3dd2663bcf09a30b84 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 24 Sep 2023 08:37:10 +0100 Subject: [PATCH 28/31] Fix build issues --- include/slang/numeric/ConstantValue.h | 2 ++ tools/netlist/include/SplitVariables.h | 4 ++-- tools/netlist/tests/VariableSelectorsTests.cpp | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/slang/numeric/ConstantValue.h b/include/slang/numeric/ConstantValue.h index 652a5fd3a..cd1a50ecf 100644 --- a/include/slang/numeric/ConstantValue.h +++ b/include/slang/numeric/ConstantValue.h @@ -311,6 +311,8 @@ struct SLANG_EXPORT ConstantRange { int32_t left = 0; int32_t right = 0; + ConstantRange(int32_t left, int32_t right) : left(left), right(right) {} + /// Gets the width of the range, regardless of the order in which /// the bounds are specified. bitwidth_t width() const { diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index fc293d840..ec986dd9c 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -49,7 +49,7 @@ class AnalyseVariableReference { return {multiplierElem * rw, fixedSizeElem}; } - SLANG_ASSERT(0 && "unsupported type for getTypeBitWidth"); + SLANG_UNREACHABLE; } /// Return the bit width of a slang type, treating unpacked arrays as @@ -107,7 +107,7 @@ class AnalyseVariableReference { return arrayType.range; } else { - SLANG_ASSERT(0 && "unexpected array type"); + SLANG_UNREACHABLE; } } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 7aa1c6ae2..91c9137e9 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -8,6 +8,7 @@ #include "NetlistTest.h" #include "SplitVariables.h" +#include "slang/util/Util.h" #include /// Helper method to extract a variable reference from a netlist and return the @@ -15,7 +16,7 @@ ConstantRange getBitRange(Netlist& netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); if (node == nullptr) { - throw std::runtime_error(fmt::format("Could not find node {}", variableSyntax)); + SLANG_THROW(fmt::format("Could not find node {}", variableSyntax)); } return AnalyseVariableReference::create(*node).getBitRange(); } From 9f425efc276f3d1ea6669ea696574f2f5f9595b0 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 24 Sep 2023 07:39:08 +0000 Subject: [PATCH 29/31] style: pre-commit fixes --- tools/netlist/include/SplitVariables.h | 2 +- tools/netlist/tests/VariableSelectorsTests.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/netlist/include/SplitVariables.h b/tools/netlist/include/SplitVariables.h index ec986dd9c..22b1bc85f 100644 --- a/tools/netlist/include/SplitVariables.h +++ b/tools/netlist/include/SplitVariables.h @@ -107,7 +107,7 @@ class AnalyseVariableReference { return arrayType.range; } else { - SLANG_UNREACHABLE; + SLANG_UNREACHABLE; } } diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 91c9137e9..57f88dcbd 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -8,15 +8,16 @@ #include "NetlistTest.h" #include "SplitVariables.h" -#include "slang/util/Util.h" #include +#include "slang/util/Util.h" + /// Helper method to extract a variable reference from a netlist and return the /// bit range associated with it. ConstantRange getBitRange(Netlist& netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); if (node == nullptr) { - SLANG_THROW(fmt::format("Could not find node {}", variableSyntax)); + SLANG_THROW(fmt::format("Could not find node {}", variableSyntax)); } return AnalyseVariableReference::create(*node).getBitRange(); } From 51abe4e33bff8815beb7e2fd172f3cb5042229c6 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 24 Sep 2023 08:44:09 +0100 Subject: [PATCH 30/31] Missing constructor --- include/slang/numeric/ConstantValue.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/slang/numeric/ConstantValue.h b/include/slang/numeric/ConstantValue.h index cd1a50ecf..6076215f6 100644 --- a/include/slang/numeric/ConstantValue.h +++ b/include/slang/numeric/ConstantValue.h @@ -311,6 +311,7 @@ struct SLANG_EXPORT ConstantRange { int32_t left = 0; int32_t right = 0; + ConstantRange() = default; ConstantRange(int32_t left, int32_t right) : left(left), right(right) {} /// Gets the width of the range, regardless of the order in which From a3aac2284f18f5bc09ab9f7195d3d9ec325ea4c9 Mon Sep 17 00:00:00 2001 From: James Hanlon Date: Sun, 24 Sep 2023 09:26:18 +0100 Subject: [PATCH 31/31] Fix exception --- tools/netlist/tests/VariableSelectorsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/netlist/tests/VariableSelectorsTests.cpp b/tools/netlist/tests/VariableSelectorsTests.cpp index 57f88dcbd..d17f87084 100644 --- a/tools/netlist/tests/VariableSelectorsTests.cpp +++ b/tools/netlist/tests/VariableSelectorsTests.cpp @@ -17,7 +17,7 @@ ConstantRange getBitRange(Netlist& netlist, std::string_view variableSyntax) { auto* node = netlist.lookupVariableReference(variableSyntax); if (node == nullptr) { - SLANG_THROW(fmt::format("Could not find node {}", variableSyntax)); + SLANG_THROW(std::runtime_error(fmt::format("Could not find node {}", variableSyntax))); } return AnalyseVariableReference::create(*node).getBitRange(); }