Skip to content

Commit

Permalink
Make incorrect soft ordering constraint into a hard constraint.
Browse files Browse the repository at this point in the history
An ordering constraint between NBA commit blocks ('Post' logic) and the
written variable were previously added as soft constraints (cutable
edges). However these are required for correctness, so if it ever is
cut we will have incorrect simulation results.

Change these into hard constraints instead. This necessitates adding a
flag on AstVar to ignore special variables constructed during V3Delayed
that might otherwise appear as degenerate logic loops. E.g.:

if (VdlySet) {
   VdlySet = 0;  // <- This write to VdlySet can and must be ignored
   LHS = VdlyVal;
}

No functional change, but you might get an error if this constraint was
ever violated. (Theoretically it should never be, as these variables
were inserted in a way that does not require violating these constraints
...)
  • Loading branch information
gezalore committed Oct 7, 2024
1 parent a257f0e commit fa4bcdb
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 16 deletions.
4 changes: 4 additions & 0 deletions src/V3AstNodeOther.h
Original file line number Diff line number Diff line change
Expand Up @@ -1847,6 +1847,7 @@ class AstVar final : public AstNode {
bool m_isForcedByCode : 1; // May be forced/released from AstAssignForce/AstRelease
bool m_isWrittenByDpi : 1; // This variable can be written by a DPI Export
bool m_isWrittenBySuspendable : 1; // This variable can be written by a suspendable process
bool m_ignorePostWrite : 1; // Ignore writes in 'Post' blocks during ordering

void init() {
m_ansi = false;
Expand Down Expand Up @@ -1891,6 +1892,7 @@ class AstVar final : public AstNode {
m_isForcedByCode = false;
m_isWrittenByDpi = false;
m_isWrittenBySuspendable = false;
m_ignorePostWrite = false;
m_attrClocker = VVarAttrClocker::CLOCKER_UNKNOWN;
}

Expand Down Expand Up @@ -2046,6 +2048,8 @@ class AstVar final : public AstNode {
void setWrittenByDpi() { m_isWrittenByDpi = true; }
bool isWrittenBySuspendable() const { return m_isWrittenBySuspendable; }
void setWrittenBySuspendable() { m_isWrittenBySuspendable = true; }
bool ignorePostWrite() const { return m_ignorePostWrite; }
void setIgnorePostWrite() { m_ignorePostWrite = true; }

// METHODS
void name(const string& name) override { m_name = name; }
Expand Down
4 changes: 3 additions & 1 deletion src/V3Delayed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ class DelayedVisitor final : public VNVisitor {

// Create new flag
AstVarScope* const flagVscp = createTemp(flp, scopep, "__VdlySet" + baseName, 1);
flagVscp->varp()->setIgnorePostWrite();
// Set the flag at the original NBA
nodep->addHereThisAsNext( //
new AstAssign{flp, new AstVarRef{flp, flagVscp, VAccess::WRITE},
Expand Down Expand Up @@ -542,6 +543,7 @@ class DelayedVisitor final : public VNVisitor {
const std::string name = "__VdlyCommitQueue" + vscp->varp()->shortName();
AstVarScope* const queueVscp = createTemp(flp, scopep, name, cqDTypep);
queueVscp->varp()->noReset(true);
queueVscp->varp()->setIgnorePostWrite();
vscpInfo.valueQueue().vscp = queueVscp;
// Create the AstActive for the Post logic
AstActive* const activep
Expand All @@ -555,7 +557,7 @@ class DelayedVisitor final : public VNVisitor {
AstCMethodHard* const callp
= new AstCMethodHard{flp, new AstVarRef{flp, queueVscp, VAccess::READWRITE}, "commit"};
callp->dtypeSetVoid();
callp->addPinsp(new AstVarRef{flp, vscp, VAccess::READWRITE});
callp->addPinsp(new AstVarRef{flp, vscp, VAccess::WRITE});
postp->addStmtsp(callp->makeStmt());
}
template <bool Partial>
Expand Down
26 changes: 11 additions & 15 deletions src/V3OrderGraphBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,24 +215,20 @@ class OrderGraphBuilder final : public VNVisitor {
// Update VarUsage
varscp->user2(varscp->user2() | VU_GEN);
// Add edges for produced variables
if (!m_inClocked || m_inPost) {
// Combinational logic
OrderVarVertex* const varVxp = getVarVertex(varscp, VarVertexType::STD);
// Add edge from producing LogicVertex -> produced VarStdVertex
if (m_inPost) {
m_graphp->addSoftEdge(m_logicVxp, varVxp, WEIGHT_COMBO);
} else {
if (m_inPost) {
if (!varscp->varp()->ignorePostWrite()) {
// Add edge from producing LogicVertex -> produced VarStdVertex
OrderVarVertex* const varVxp = getVarVertex(varscp, VarVertexType::STD);
m_graphp->addHardEdge(m_logicVxp, varVxp, WEIGHT_NORMAL);
}

OrderVarVertex* const postVxp = getVarVertex(varscp, VarVertexType::POST);
// Add edge from produced VarPostVertex -> to producing LogicVertex
m_graphp->addHardEdge(postVxp, m_logicVxp, WEIGHT_POST);
} else if (!m_inClocked) { // Combinational logic
// Add edge from producing LogicVertex -> produced VarStdVertex
OrderVarVertex* const varVxp = getVarVertex(varscp, VarVertexType::STD);
m_graphp->addHardEdge(m_logicVxp, varVxp, WEIGHT_NORMAL);
// Add edge from produced VarPostVertex -> to producing LogicVertex

// For m_inPost:
// Add edge consumed_var_POST->logic_vertex
// This prevents a consumer of the "early" value to be scheduled
// after we've changed to the next-cycle value
// ALWAYS do it:
// There maybe a wire a=b; between the two blocks
OrderVarVertex* const postVxp = getVarVertex(varscp, VarVertexType::POST);
m_graphp->addHardEdge(postVxp, m_logicVxp, WEIGHT_POST);
} else if (m_inPre) { // AstAssignPre
Expand Down

0 comments on commit fa4bcdb

Please sign in to comment.