[slang] Introduce net aliases bits duplication checks (#1045)

include/slang/ast/Compilation.h

@@ -711,6 +711,11 @@ class SLANG_EXPORT Compilation : public BumpAllocator {
     /// Queries if any errors have been issued on any scope within this compilation.
     bool hasIssuedErrors() const { return numErrors > 0; };
 
+    /// Check that provided diagnotic code and location with it's note is exists in the compilation
+    /// diagnostic scope
+    bool checkDiagAndNote(DiagCode diagCode, SourceLocation diagLoc, DiagCode noteCode,
+                          SourceLocation noteLoc);
+
     /// @{
 
 private:

include/slang/ast/SemanticFacts.h

@@ -108,7 +108,7 @@ SLANG_ENUM(ForwardTypeRestriction, FTR);
 #undef FTR
 
 /// A set of flags that control how assignments are checked.
-enum class SLANG_EXPORT AssignFlags : uint8_t {
+enum class SLANG_EXPORT AssignFlags : uint16_t {
     /// No special assignment behavior specified.
     None = 0,
 
@@ -137,9 +137,12 @@ enum class SLANG_EXPORT AssignFlags : uint8_t {
 
     /// The assignment is for an output port that was sliced due to an array of instances
     /// being connected to an array argument.
-    SlicedPort = 1 << 7
+    SlicedPort = 1 << 7,
+
+    /// The assignment declares net alias
+    NetAlias = 1 << 8
 };
-SLANG_BITMASK(AssignFlags, SlicedPort)
+SLANG_BITMASK(AssignFlags, NetAlias)
 
 /// A helper class that can extract semantic AST information from
 /// tokens and syntax nodes.

include/slang/ast/symbols/ValueSymbol.h

@@ -64,6 +64,10 @@ class SLANG_EXPORT ValueSymbol : public Symbol {
     void addDriver(DriverKind kind, const Expression& longestStaticPrefix,
                    const Symbol& containingSymbol, bitmask<AssignFlags> flags) const;
 
+    void addDriver(DriverKind kind, const Expression& longestStaticPrefix,
+                   const Symbol& containingSymbol, bitmask<AssignFlags> flags,
+                   const SourceRange& symExprSR, DriverBitRange exprBounds) const;
+
     void addDriver(DriverKind kind, DriverBitRange bounds, const Expression& longestStaticPrefix,
                    const Symbol& containingSymbol, const Expression& procCallExpression) const;
 
@@ -118,11 +122,15 @@ class SLANG_EXPORT ValueDriver {
     /// The kind of driver (procedural or continuous).
     DriverKind kind;
 
+    /// Stored driven value expression source range for diagnostic
+    const SourceRange* symExprSR = nullptr;
+
     /// Constructs a new ValueDriver instance.
     ValueDriver(DriverKind kind, const Expression& longestStaticPrefix,
-                const Symbol& containingSymbol, bitmask<AssignFlags> flags) :
+                const Symbol& containingSymbol, bitmask<AssignFlags> flags,
+                const SourceRange* symExprSR = nullptr) :
         prefixExpression(&longestStaticPrefix), containingSymbol(&containingSymbol), flags(flags),
-        kind(kind) {}
+        kind(kind), symExprSR(symExprSR) {}
 
     /// Indicates whether the driver is for an input port.
     bool isInputPort() const { return flags.has(AssignFlags::InputPort); }
@@ -138,6 +146,9 @@ class SLANG_EXPORT ValueDriver {
     /// Indicates whether the driver is for an assertion local variable formal argument.
     bool isLocalVarFormalArg() const { return flags.has(AssignFlags::AssertionLocalVarFormalArg); }
 
+    /// Indicates whether the driver is for a net alias.
+    bool isNetAlias() const { return flags.has(AssignFlags::NetAlias); }
+
     /// Indicates whether the driver is inside a single-driver procedure (such as always_comb).
     bool isInSingleDriverProcedure() const;
 

scripts/diagnostics.txt

@@ -9,6 +9,8 @@ note NoteDeclarationHere "declared here"
 note NotePreviousMatch "previous match here"
 note NoteReferencedHere "referenced here"
 note NoteAssignedHere "also assigned here"
+note NoteAliasedTo "which is aliased to"
+note NoteAliasDeclaration "previous alias declaration here"
 note NoteDrivenHere "driven here"
 note NoteOriginalAssign "original assignment here"
 note NoteConfigRule "from configuration here"
@@ -715,6 +717,7 @@ error MultipleContAssigns "cannot have multiple continuous assignments to variab
 error MultipleAlwaysAssigns "variable '{}' driven by {} procedure cannot be written to by any other process"
 error MultipleUWireDrivers "'uwire' net '{}' cannot have multiple drivers"
 error MultipleUDNTDrivers "net '{}' with user-defined nettype '{}' cannot have multiple drivers because it does not specify a resolution function"
+error MultipleNetAlias "it is not allowed to specify an alias from an individual signal to itself or to specify a given alias more than once"
 error InputPortAssign "cannot assign to input port '{}'"
 error ClockVarTargetAssign "cannot write to '{}' with a continuous assignment since it's associated with an output clocking signal"
 error LocalFormalVarMultiAssign "cannot bind local variable '{}' to more than one output or inout local variable formal argument"

source/ast/Compilation.cpp

@@ -1633,6 +1633,20 @@ Diagnostic& Compilation::addDiag(Diagnostic diag) {
     return it->second.back();
 }
 
+bool Compilation::checkDiagAndNote(DiagCode diagCode, SourceLocation diagLoc, DiagCode noteCode,
+                                   SourceLocation noteLoc) {
+    if (auto diagIt = diagMap.find({diagCode, diagLoc}); diagIt != diagMap.end()) {
+        for (auto& diag : diagIt->second) {
+            for (auto& note : diag.notes) {
+                if (note.code == noteCode && note.location == noteLoc)
+                    return true;
+            }
+        }
+    }
+
+    return false;
+}
+
 AssertionInstanceDetails* Compilation::allocAssertionDetails() {
     return assertionDetailsAllocator.emplace();
 }

source/ast/symbols/MemberSymbols.cpp

@@ -2392,12 +2392,21 @@ NetAliasSymbol& NetAliasSymbol::fromSyntax(const ASTContext& parentContext,
     return *result;
 }
 
+struct NetAliasPOD {
+    const ValueSymbol* sym;
+    const Expression* expr;
+    std::optional<DriverBitRange> bounds;
+};
+
 struct NetAliasVisitor {
     const ASTContext& context;
     const NetType* commonNetType = nullptr;
     bool issuedError = false;
+    SmallVector<NetAliasPOD, 4> netAliases;
+    EvalContext& evalCtx;
 
-    NetAliasVisitor(const ASTContext& context) : context(context) {}
+    NetAliasVisitor(const ASTContext& context, EvalContext& evalCtx) :
+        context(context), evalCtx(evalCtx) {}
 
     template<typename T>
     void visit(const T& expr) {
@@ -2412,7 +2421,11 @@ struct NetAliasVisitor {
                             context.addDiag(diag::NetAliasNotANet, expr.sourceRange) << sym->name;
                         }
                         else {
-                            auto& nt = sym->template as<NetSymbol>().netType;
+                            auto& netSym = sym->template as<NetSymbol>();
+                            netAliases.push_back(
+                                {&netSym, &expr,
+                                 ValueDriver::getBounds(expr, evalCtx, netSym.getType())});
+                            auto& nt = netSym.netType;
                             if (!commonNetType) {
                                 commonNetType = &nt;
                             }
@@ -2447,14 +2460,14 @@ std::span<const Expression* const> NetAliasSymbol::getNetReferences() const {
     auto syntax = getSyntax();
     SLANG_ASSERT(scope && syntax);
 
-    // TODO: there should be a global check somewhere that any given bit
-    // of a net isn't aliased to the same target signal bit multiple times.
     bitwidth_t bitWidth = 0;
     bool issuedError = false;
     SmallVector<const Expression*> buffer;
     ASTContext context(*scope, LookupLocation::after(*this),
                        ASTFlags::NonProcedural | ASTFlags::NotADriver);
-    NetAliasVisitor visitor(context);
+    EvalContext evalCtx(ASTContext(*scope, LookupLocation::max));
+    NetAliasVisitor visitor(context, evalCtx);
+    SmallVector<std::pair<const Expression*, std::span<NetAliasPOD>>, 4> netAliases;
 
     for (auto exprSyntax : syntax->as<NetAliasSyntax>().nets) {
         auto& netRef = Expression::bind(*exprSyntax, context);
@@ -2472,9 +2485,83 @@ std::span<const Expression* const> NetAliasSymbol::getNetReferences() const {
 
         netRef.visit(visitor);
         buffer.push_back(&netRef);
+        if (!visitor.netAliases.empty()) {
+            netAliases.push_back(
+                std::make_pair(&netRef, visitor.netAliases.copy(scope->getCompilation())));
+            visitor.netAliases.clear();
+        }
     }
 
     netRefs = buffer.copy(scope->getCompilation());
+
+    // To process an each pair a single time.
+    SmallSet<const Expression*, 4> processed;
+    // Go through the list of net aliases with a sliding window adding each alias in pairs to each
+    // as a driver. Along the way, calculating the subexpessions bounds (there is no duplication of
+    // calculations, since the bounds are calculated only once for each subexpression.) and bit
+    // remainders (which will be processed on the next iterations), since the subexpressions can be
+    // of different bit lengths.
+    for (auto first : netAliases) {
+        processed.insert(first.first);
+        for (auto second : netAliases) {
+            if (processed.contains(second.first))
+                continue;
+
+            auto currFirst = first.second.begin();
+            auto endFirst = first.second.end();
+            auto currSecond = second.second.begin();
+            auto endSecond = second.second.end();
+            // boolean member means bit range remainder of left (if it is true) or right otherwise
+            std::optional<std::pair<DriverBitRange, bool>> remainder = std::nullopt;
+            while (currFirst != endFirst && currSecond != endSecond) {
+                auto rangeFirst = currFirst->bounds.value_or(
+                    DriverBitRange(0, currFirst->expr->type->getBitWidth()));
+                auto rangeSecond = currSecond->bounds.value_or(
+                    DriverBitRange(0, currSecond->expr->type->getBitWidth()));
+
+                if (remainder.has_value()) {
+                    if (remainder.value().second)
+                        rangeFirst = remainder.value().first;
+                    else
+                        rangeSecond = remainder.value().first;
+
+                    remainder = std::nullopt;
+                }
+
+                auto rangeDiff = std::abs(int64_t(rangeFirst.second - rangeFirst.first)) -
+                                 std::abs(int64_t(rangeSecond.second - rangeSecond.first));
+                auto currFirstSaved = currFirst;
+                auto currSecondSaved = currSecond;
+                if (rangeDiff > 0) {
+                    int64_t newBound = (int64_t)rangeFirst.second - rangeDiff + 1;
+                    SLANG_ASSERT(newBound >= 0);
+                    remainder =
+                        std::make_pair(DriverBitRange((uint64_t)newBound, rangeFirst.second), true);
+                    rangeFirst = DriverBitRange(rangeFirst.first, (uint64_t)newBound - 1);
+                    ++currSecond;
+                }
+                else if (rangeDiff < 0) {
+                    int64_t newBound = (int64_t)rangeSecond.second + rangeDiff + 1;
+                    SLANG_ASSERT(newBound >= 0);
+                    remainder = std::make_pair(
+                        DriverBitRange((uint64_t)newBound, rangeSecond.second), false);
+                    rangeSecond = DriverBitRange(rangeSecond.first, (uint64_t)newBound - 1);
+                    ++currFirst;
+                }
+                else {
+                    ++currFirst;
+                    ++currSecond;
+                }
+
+                currFirstSaved->sym->addDriver(DriverKind::Continuous, *currSecondSaved->expr,
+                                               *currSecondSaved->sym, AssignFlags::NetAlias,
+                                               currFirstSaved->expr->sourceRange, rangeSecond);
+                currSecondSaved->sym->addDriver(DriverKind::Continuous, *currFirstSaved->expr,
+                                                *currFirstSaved->sym, AssignFlags::NetAlias,
+                                                currSecondSaved->expr->sourceRange, rangeFirst);
+            }
+        }
+    }
     return *netRefs;
 }
 

source/ast/symbols/ValueSymbol.cpp

@@ -171,19 +171,34 @@ static bool handleOverlap(const Scope& scope, std::string_view name, const Value
         code = diag::MultipleUWireDrivers;
     else if (isSingleDriverUDNT)
         code = diag::MultipleUDNTDrivers;
+    else if (driver.isNetAlias())
+        code = diag::MultipleNetAlias;
     else if (driver.kind == DriverKind::Continuous && curr.kind == DriverKind::Continuous)
         code = diag::MultipleContAssigns;
     else
         code = diag::MixedVarAssigns;
 
+    // Filter out duplicate diagnostics with accuracy up to swap diagnostic and note.
+    if (driver.isNetAlias() &&
+        scope.getCompilation().checkDiagAndNote(diag::MultipleNetAlias, currRange.start(),
+                                                diag::NoteAliasDeclaration, driverRange.start()))
+        return false;
+
     auto& diag = scope.addDiag(code, driverRange);
     diag << name;
     if (isSingleDriverUDNT) {
         SLANG_ASSERT(netType);
         diag << netType->name;
     }
 
-    addAssignedHereNote(diag);
+    if (!driver.isNetAlias()) {
+        addAssignedHereNote(diag);
+    }
+    else {
+        diag.addNote(diag::NoteAliasedTo, *driver.symExprSR);
+        diag.addNote(diag::NoteAliasDeclaration, currRange);
+        diag.addNote(diag::NoteAliasedTo, *curr.symExprSR);
+    }
     return false;
 }
 
@@ -204,6 +219,15 @@ void ValueSymbol::addDriver(DriverKind driverKind, const Expression& longestStat
     addDriver(*bounds, *driver);
 }
 
+void ValueSymbol::addDriver(DriverKind driverKind, const Expression& longestStaticPrefix,
+                            const Symbol& containingSymbol, bitmask<AssignFlags> flags,
+                            const SourceRange& symExprSR, DriverBitRange exprBounds) const {
+    auto& comp = getParentScope()->getCompilation();
+    auto driver = comp.emplace<ValueDriver>(driverKind, longestStaticPrefix, containingSymbol,
+                                            flags, &symExprSR);
+    addDriver(exprBounds, *driver);
+}
+
 void ValueSymbol::addDriver(DriverKind driverKind, DriverBitRange bounds,
                             const Expression& longestStaticPrefix, const Symbol& containingSymbol,
                             const Expression& procCallExpression) const {
@@ -299,6 +323,7 @@ void ValueSymbol::addDriver(DriverBitRange bounds, const ValueDriver& driver) co
         //            block to overlap even if the other block is an always_comb/ff.
         // - Assertion local variable formal arguments can't drive more than
         //   one output to the same local variable.
+        // - Net bits are not aliased more than once
         bool isProblem = false;
         auto curr = *it;
 
@@ -326,6 +351,13 @@ void ValueSymbol::addDriver(DriverBitRange bounds, const ValueDriver& driver) co
             }
         }
 
+        // If one of the drivers is an alias, then perform a check if the second one is an alias
+        if (curr->isNetAlias() || driver.isNetAlias()) {
+            isProblem = curr->isNetAlias() && driver.isNetAlias();
+            // Check that all net alias drivers are have the same net alias symbol scope
+            isProblem = isProblem && (curr->containingSymbol == driver.containingSymbol);
+        }
+
         if (isProblem) {
             if (!handleOverlap(*scope, name, *curr, driver, isNet, isUWire, isSingleDriverUDNT,
                                netType)) {