Internals: Make AstCExpr always cleanOut (#6280) (#6570)

There was exactly one place in V3Task, handling DPI arguments when we
relied on cleanOut of AstCExpr being false for masking. Made that code
do the relevant masking via a few new run-time functions, which also
eliminates some special cases in the relevant V3Task functions.
This commit is contained in:
Geza Lore 2025-10-19 10:44:33 +02:00 committed by GitHub
parent 4ef6f00423
commit 61c64e4a3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 123 additions and 137 deletions

View File

@ -42,10 +42,18 @@ static inline void VL_SET_W_SVBV(int obits, WDataOutP owp, const svBitVecVal* lw
for (int i = 0; i < words - 1; ++i) owp[i] = lwp[i]; for (int i = 0; i < words - 1; ++i) owp[i] = lwp[i];
owp[words - 1] = lwp[words - 1] & VL_MASK_I(obits); owp[words - 1] = lwp[words - 1] & VL_MASK_I(obits);
} }
static inline QData VL_SET_Q_SVBV(const svBitVecVal* lwp) VL_MT_SAFE { static inline void VL_SET_Q_SVBV(int obits, QData& out, const svBitVecVal* lwp) VL_MT_SAFE {
return VL_SET_QII(lwp[1], lwp[0]); out = VL_MASK_Q(obits) & VL_SET_QII(lwp[1], lwp[0]);
}
static inline void VL_SET_I_SVBV(int obits, IData& out, const svBitVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0];
}
static inline void VL_SET_S_SVBV(int obits, SData& out, const svBitVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0];
}
static inline void VL_SET_C_SVBV(int obits, CData& out, const svBitVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0];
} }
static inline IData VL_SET_I_SVBV(const svBitVecVal* lwp) VL_MT_SAFE { return lwp[0]; }
// Convert Verilator internal data to svBitVecVal // Convert Verilator internal data to svBitVecVal
static inline void VL_SET_SVBV_W(int obits, svBitVecVal* owp, const WDataInP lwp) VL_MT_SAFE { static inline void VL_SET_SVBV_W(int obits, svBitVecVal* owp, const WDataInP lwp) VL_MT_SAFE {
@ -65,10 +73,18 @@ static inline void VL_SET_W_SVLV(int obits, WDataOutP owp, const svLogicVecVal*
for (int i = 0; i < words - 1; ++i) owp[i] = lwp[i].aval; for (int i = 0; i < words - 1; ++i) owp[i] = lwp[i].aval;
owp[words - 1] = lwp[words - 1].aval & VL_MASK_I(obits); owp[words - 1] = lwp[words - 1].aval & VL_MASK_I(obits);
} }
static inline QData VL_SET_Q_SVLV(const svLogicVecVal* lwp) VL_MT_SAFE { static inline void VL_SET_Q_SVLV(int obits, QData& out, const svLogicVecVal* lwp) VL_MT_SAFE {
return VL_SET_QII(lwp[1].aval, lwp[0].aval); out = VL_MASK_Q(obits) & VL_SET_QII(lwp[1].aval, lwp[0].aval);
}
static inline void VL_SET_I_SVLV(int obits, IData& out, const svLogicVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0].aval;
}
static inline void VL_SET_S_SVLV(int obits, SData& out, const svLogicVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0].aval;
}
static inline void VL_SET_C_SVLV(int obits, CData& out, const svLogicVecVal* lwp) VL_MT_SAFE {
out = VL_MASK_I(obits) & lwp[0].aval;
} }
static inline IData VL_SET_I_SVLV(const svLogicVecVal* lwp) VL_MT_SAFE { return lwp[0].aval; }
// Convert Verilator internal data to svLogicVecVal // Convert Verilator internal data to svLogicVecVal
// Note these functions never create X/Z in svLogicVecVal // Note these functions never create X/Z in svLogicVecVal

View File

@ -159,9 +159,8 @@ AstCStmt::AstCStmt(FileLine* fl, const string& textStmt)
addExprsp(new AstText{fl, textStmt, true}); addExprsp(new AstText{fl, textStmt, true});
} }
AstCExpr::AstCExpr(FileLine* fl, const string& textStmt, int setwidth, bool cleanOut) AstCExpr::AstCExpr(FileLine* fl, const string& textStmt, int setwidth)
: ASTGEN_SUPER_CExpr(fl) : ASTGEN_SUPER_CExpr(fl)
, m_cleanOut{cleanOut}
, m_pure{true} { , m_pure{true} {
addExprsp(new AstText{fl, textStmt, true}); addExprsp(new AstText{fl, textStmt, true});
if (setwidth) dtypeSetLogicSized(setwidth, VSigning::UNSIGNED); if (setwidth) dtypeSetLogicSized(setwidth, VSigning::UNSIGNED);

View File

@ -564,22 +564,20 @@ public:
}; };
class AstCExpr final : public AstNodeExpr { class AstCExpr final : public AstNodeExpr {
// @astgen op1 := exprsp : List[AstNode] // Expressions to print // @astgen op1 := exprsp : List[AstNode] // Expressions to print
const bool m_cleanOut;
bool m_pure; // Pure optimizable bool m_pure; // Pure optimizable
public: public:
// Emit C textual expr function (like AstUCFunc) // Emit C textual expr function (like AstUCFunc)
AstCExpr(FileLine* fl, AstNode* exprsp) AstCExpr(FileLine* fl, AstNode* exprsp)
: ASTGEN_SUPER_CExpr(fl) : ASTGEN_SUPER_CExpr(fl)
, m_cleanOut{true}
, m_pure{false} { , m_pure{false} {
addExprsp(exprsp); addExprsp(exprsp);
dtypeFrom(exprsp); dtypeFrom(exprsp);
} }
inline AstCExpr(FileLine* fl, const string& textStmt, int setwidth, bool cleanOut = true); inline AstCExpr(FileLine* fl, const string& textStmt, int setwidth);
ASTGEN_MEMBERS_AstCExpr; ASTGEN_MEMBERS_AstCExpr;
bool isGateOptimizable() const override { return m_pure; } bool isGateOptimizable() const override { return m_pure; }
bool isPredictOptimizable() const override { return m_pure; } bool isPredictOptimizable() const override { return m_pure; }
bool cleanOut() const override { return m_cleanOut; } bool cleanOut() const override { return true; }
string emitVerilog() override { V3ERROR_NA_RETURN(""); } string emitVerilog() override { V3ERROR_NA_RETURN(""); }
string emitC() override { V3ERROR_NA_RETURN(""); } string emitC() override { V3ERROR_NA_RETURN(""); }
bool sameNode(const AstNode* /*samep*/) const override { return true; } bool sameNode(const AstNode* /*samep*/) const override { return true; }

View File

@ -453,7 +453,7 @@ void orderSequentially(AstCFunc* funcp, const LogicByScope& lbs) {
subFuncp->slow(false); subFuncp->slow(false);
FileLine* const flp = procp->fileline(); FileLine* const flp = procp->fileline();
AstNodeExpr* const condp = new AstCExpr{ AstNodeExpr* const condp = new AstCExpr{
flp, "VL_LIKELY(!vlSymsp->_vm_contextp__->gotFinish())", 1, true}; flp, "VL_LIKELY(!vlSymsp->_vm_contextp__->gotFinish())", 1};
AstLoop* const loopp = new AstLoop{flp}; AstLoop* const loopp = new AstLoop{flp};
loopp->addStmtsp(new AstLoopTest{flp, loopp, condp}); loopp->addStmtsp(new AstLoopTest{flp, loopp, condp});
loopp->addStmtsp(bodyp); loopp->addStmtsp(bodyp);

View File

@ -289,46 +289,50 @@ public:
// DPI related utility functions // DPI related utility functions
struct TaskDpiUtils final { struct TaskDpiUtils final {
static std::vector<std::pair<AstUnpackArrayDType*, int>> // Returns a vector of ('elements', 'stride') pairs for each unpacked dimension
unpackDimsAndStrides(AstNodeDType* dtypep) { static std::vector<std::pair<int, int>> unpackDimsAndStrides(AstVar* varp) {
std::vector<std::pair<AstUnpackArrayDType*, int>> dimStrides; AstNodeDType* const dtypep = varp->dtypep()->skipRefp();
if (AstUnpackArrayDType* const unpackp = VN_CAST(dtypep->skipRefp(), UnpackArrayDType)) { std::vector<std::pair<int, int>> dimStrides;
AstUnpackArrayDType* const unpackp = VN_CAST(dtypep->skipRefp(), UnpackArrayDType);
if (!unpackp) return dimStrides;
const std::vector<AstUnpackArrayDType*> dims = unpackp->unpackDimensions(); const std::vector<AstUnpackArrayDType*> dims = unpackp->unpackDimensions();
dimStrides.resize(dims.size(), {nullptr, 0}); const size_t nDims = dims.size();
dimStrides.back() = {dims.back(), 1}; dimStrides.resize(nDims);
for (ssize_t i = dims.size() - 2; i >= 0; --i) { // Stride of fastest varying dimension is 1.
dimStrides[i].first = dims[i]; dimStrides[nDims - 1].first = dims.back()->elementsConst();
dimStrides[i].second = dimStrides[i + 1].second * dims[i + 1]->elementsConst(); dimStrides[nDims - 1].second = 1;
} // Rest are densly packed
for (ssize_t i = nDims - 2; i >= 0; --i) {
dimStrides[i].first = dims[i]->elementsConst();
dimStrides[i].second = dimStrides[i + 1].first * dimStrides[i + 1].second;
} }
return dimStrides; return dimStrides;
} }
static bool dpiToInternalFrStmt(AstVar* portp, const string& frName, string& frstmt, // Returns the prefix of a function-call like statement used to convert
string& ket) { // from IEEE DPI data types to internal types, and a bool that is true
ket.clear(); // if type uses svBitVecVal/svLogicVecVal and ther result is returned via
if (portp->basicp() && portp->basicp()->keyword() == VBasicDTypeKwd::CHANDLE) { // an output parameter, o false if uses C primitive type and result is the
frstmt = "VL_CVT_VP_Q(" + frName; // return value.
ket = ")"; static std::pair<std::string, bool> dpiToInternalCvtStmt(AstVar* varp) {
} else if (portp->basicp() && portp->basicp()->keyword() == VBasicDTypeKwd::STRING) { AstBasicDType* const basicp = varp->basicp();
frstmt = "VL_CVT_N_CSTR(" + frName;
ket = ")"; // DPI types using Primitive C types
} else if ((portp->basicp() && portp->basicp()->isDpiPrimitive())) { if (basicp->keyword() == VBasicDTypeKwd::CHANDLE) return {"VL_CVT_VP_Q(", false};
frstmt = frName; if (basicp->keyword() == VBasicDTypeKwd::STRING) return {"VL_CVT_N_CSTR(", false};
} else { if (basicp->isDpiPrimitive()) return {"(", false};
const string frSvType = portp->basicp()->isDpiBitVec() ? "SVBV" : "SVLV";
if (portp->isWide()) { // DPI types using svBitVecVal/svLogicVecVal
// Need to convert to wide, using special function UASSERT_OBJ(basicp->isDpiBitVec() || basicp->isDpiLogicVec(), varp,
frstmt = "VL_SET_W_" + frSvType + "(" + cvtToStr(portp->width()) + ", "; "Should use svBitVecVal/svLogicVecVal");
return true;
} else { const std::string vecType = basicp->isDpiBitVec() ? "SVBV" : "SVLV";
const AstNodeDType* const dtypep = portp->dtypep()->skipRefp(); const AstNodeDType* const dtypep = varp->dtypep()->skipRefp();
frstmt = "VL_SET_" + string{dtypep->charIQWN()} + "_" + frSvType + "("; const char sizeChar = dtypep->width() <= 8 ? 'C'
if (VN_IS(dtypep, UnpackArrayDType)) frstmt += "&"; : dtypep->width() <= 16 ? 'S'
frstmt += frName; : *dtypep->charIQWN();
ket = ")"; const std::string& size = std::to_string(dtypep->width());
} return {"VL_SET_"s + sizeChar + "_" + vecType + "(" + size + ", ", true};
}
return false;
} }
}; };
@ -673,8 +677,8 @@ class TaskVisitor final : public VNVisitor {
UASSERT_OBJ(snp, refp, "Missing scoping context"); UASSERT_OBJ(snp, refp, "Missing scoping context");
ccallp->addArgsp(snp); ccallp->addArgsp(snp);
// __Vfilenamep // __Vfilenamep
ccallp->addArgsp(new AstCExpr{ ccallp->addArgsp(
refp->fileline(), "\"" + refp->fileline()->filenameEsc() + "\"", 64, true}); new AstCExpr{refp->fileline(), "\"" + refp->fileline()->filenameEsc() + "\"", 64});
// __Vlineno // __Vlineno
ccallp->addArgsp(new AstConst(refp->fileline(), refp->fileline()->lineno())); ccallp->addArgsp(new AstConst(refp->fileline(), refp->fileline()->lineno()));
} }
@ -770,67 +774,48 @@ class TaskVisitor final : public VNVisitor {
return new AstCStmt{portp->fileline(), stmt}; return new AstCStmt{portp->fileline(), stmt};
} }
AstNode* createAssignDpiToInternal(AstVarScope* portvscp, const string& frName) { AstNodeStmt* createAssignDpiToInternal(AstVarScope* vscp, const std::string& rhsName) {
// Create assignment from DPI temporary into internal format // Create assignment from DPI temporary into internal format
// DPI temporary is scalar or 1D array (if unpacked array) // DPI temporary is scalar or 1D array (if unpacked array)
// Internal representation is scalar, 1D, or multi-dimensional array (similar to SV) // Internal representation is scalar, 1D, or multi-dimensional array (similar to SV)
AstVar* const portp = portvscp->varp(); AstVar* const varp = vscp->varp();
string frstmt; FileLine* const flp = vscp->fileline();
string ket; std::string cvt;
const bool useSetWSvlv = TaskDpiUtils::dpiToInternalFrStmt(portp, frName, frstmt, ket); bool useSvVec;
// Use a AstCExpr, as we want V3Clean to mask off bits that don't make sense. std::tie(cvt, useSvVec) = TaskDpiUtils::dpiToInternalCvtStmt(varp);
int cwidth = VL_IDATASIZE; const std::vector<std::pair<int, int>> strides = TaskDpiUtils::unpackDimsAndStrides(varp);
if (!useSetWSvlv && portp->basicp()) { // Total number of elements in unpacked array
if (portp->basicp()->keyword().isBitLogic()) { const int total = strides.empty() ? 1 : strides[0].first * strides[0].second;
cwidth = VL_EDATASIZE * portp->widthWords(); // Number of words per element/primitive type
} else { const int widthWords = varp->basicp()->widthWords();
cwidth = portp->basicp()->keyword().width(); // Number of bits in the C expression result, for masking.
} const int cwidth = widthWords * VL_EDATASIZE;
// The resulting list of statements
AstNodeStmt* stmtsp = nullptr;
for (int i = 0; i < total; ++i) {
AstNodeExpr* lhsp = new AstVarRef{flp, vscp, VAccess::WRITE};
// Extract a scalar from multi-dimensional array (internal format)
for (const auto& stride : strides) {
lhsp = new AstArraySel(flp, lhsp, (i / stride.second) % stride.first);
} }
const std::vector<std::pair<AstUnpackArrayDType*, int>> dimStrides // Extract a scalar from DPI temporary var that is scalar or 1D array
= TaskDpiUtils::unpackDimsAndStrides(portp->dtypep()); if (useSvVec) {
const int total = dimStrides.empty() ? 1 const std::string offset = std::to_string(i * widthWords);
: dimStrides.front().first->elementsConst() AstCStmt* const cstmtp = new AstCStmt{flp, nullptr};
* dimStrides.front().second; cstmtp->addExprsp(new AstText{flp, cvt});
AstNode* newp = nullptr; cstmtp->addExprsp(lhsp);
const int widthWords = portp->basicp()->widthWords(); cstmtp->addExprsp(new AstText{flp, ", " + rhsName + " + " + offset + ");"});
for (int i = 0; i < total; ++i) { stmtsp = AstNode::addNext(stmtsp, cstmtp);
AstNodeExpr* srcp = new AstVarRef{portvscp->fileline(), portvscp, VAccess::WRITE};
// extract a scalar from multi-dimensional array (internal format)
for (auto&& dimStride : dimStrides) {
const size_t dimIdx = (i / dimStride.second) % dimStride.first->elementsConst();
srcp = new AstArraySel(portvscp->fileline(), srcp, dimIdx);
}
AstNode* stmtp = nullptr;
// extract a scalar from DPI temporary var that is scalar or 1D array
if (useSetWSvlv) {
AstNode* const linesp = new AstText{portvscp->fileline(), frstmt + ket};
linesp->addNext(srcp);
linesp->addNext(
new AstText{portvscp->fileline(),
", " + frName + " + " + cvtToStr(i * widthWords) + ");"});
stmtp = new AstCStmt{portvscp->fileline(), linesp};
} else { } else {
string from = frstmt; const std::string elem = strides.empty() ? "" : "[" + std::to_string(i) + "]";
if (!dimStrides.empty()) { AstNodeExpr* rhsp = new AstCExpr{flp, cvt + rhsName + elem + ")", cwidth};
// e.g. time is 64bit svLogicVector rhsp = new AstSel{flp, rhsp, 0, varp->width()};
const int coef = portp->basicp()->isDpiLogicVec() ? widthWords : 1; stmtsp = AstNode::addNext(stmtsp, new AstAssign{flp, lhsp, rhsp});
from += "[" + cvtToStr(i * coef) + "]";
}
from += ket;
AstNodeExpr* const rhsp = new AstSel{
portp->fileline(), new AstCExpr{portp->fileline(), from, cwidth, false}, 0,
portp->width()};
stmtp = new AstAssign{portp->fileline(), srcp, rhsp};
}
if (i > 0) {
newp->addNext(stmtp);
} else {
newp = stmtp;
} }
} }
return newp; return stmtsp;
} }
AstCFunc* makeDpiExportDispatcher(AstNodeFTask* nodep, AstVar* rtnvarp) { AstCFunc* makeDpiExportDispatcher(AstNodeFTask* nodep, AstVar* rtnvarp) {
@ -2024,42 +2009,30 @@ string V3Task::assignInternalToDpi(AstVar* portp, bool isPtr, const string& frSu
return stmt; return stmt;
} }
string V3Task::assignDpiToInternal(const string& lhsName, AstVar* varp) { // Create assignment from DPI temporary into internal format
// Create assignment from DPI temporary into internal format // DPI temporary is scalar or 1D array (if unpacked array)
// DPI temporary is scalar or 1D array (if unpacked array) // Internal representation is scalar, 1D, or multi-dimensional array (similar to SV)
// Internal representation is scalar, 1D, or multi-dimensional array (similar to SV) std::string V3Task::assignDpiToInternal(const std::string& lhsName, AstVar* varp) {
const string frName = varp->name(); std::string cvt;
string frstmt; bool useSvVec;
string ket; std::tie(cvt, useSvVec) = TaskDpiUtils::dpiToInternalCvtStmt(varp);
const bool useSetWSvlv = TaskDpiUtils::dpiToInternalFrStmt(varp, frName, frstmt, ket); const std::vector<std::pair<int, int>> strides = TaskDpiUtils::unpackDimsAndStrides(varp);
const int total = strides.empty() ? 1 : strides[0].first * strides[0].second;
const std::vector<std::pair<AstUnpackArrayDType*, int>> dimStrides
= TaskDpiUtils::unpackDimsAndStrides(varp->dtypep());
const int total = dimStrides.empty()
? 1
: dimStrides.front().first->elementsConst() * dimStrides.front().second;
const int widthWords = varp->basicp()->widthWords(); const int widthWords = varp->basicp()->widthWords();
string statements; string statements;
for (int i = 0; i < total; ++i) { for (int i = 0; i < total; ++i) {
string lhs = lhsName; std::string lhs = lhsName;
// extract a scalar from multi-dimensional array (internal format) // extract a scalar from multi-dimensional array (internal format)
for (auto&& dimStride : dimStrides) { for (const auto& stride : strides) {
const size_t dimIdx = (i / dimStride.second) % dimStride.first->elementsConst(); lhs += "[" + std::to_string((i / stride.second) % stride.first) + "]";
lhs += "[" + cvtToStr(dimIdx) + "]";
} }
// extract a scalar from DPI temporary var that is scalar or 1D array // extract a scalar from DPI temporary var that is scalar or 1D array
if (useSetWSvlv) { if (useSvVec) {
statements += frstmt + ket + " " + lhs + ", " + frName + " + " const std::string offset = std::to_string(i * widthWords);
+ cvtToStr(i * widthWords) + ");\n"; statements += cvt + " " + lhs + ", " + varp->name() + " + " + offset + ");\n";
} else { } else {
string rhs = frstmt; const std::string elem = strides.empty() ? "" : "[" + std::to_string(i) + "]";
if (!dimStrides.empty()) { statements += lhs + " = " + cvt + varp->name() + elem + ")" + ";\n";
// e.g. time is 64bit svLogicVector
const int coef = varp->basicp()->isDpiLogicVec() ? widthWords : 1;
rhs += "[" + cvtToStr(i * coef) + "]";
}
rhs += ket;
statements += lhs + " = " + rhs + ";\n";
} }
} }
return statements; return statements;

View File

@ -1186,7 +1186,7 @@ class TimingControlVisitor final : public VNVisitor {
if (constp->isZero()) { if (constp->isZero()) {
// We have to await forever instead of simply returning in case we're deep in a // We have to await forever instead of simply returning in case we're deep in a
// callstack // callstack
AstCExpr* const foreverp = new AstCExpr{flp, "VlForever{}", 0, true}; AstCExpr* const foreverp = new AstCExpr{flp, "VlForever{}", 0};
foreverp->dtypeSetVoid(); // TODO: this is sloppy but harmless foreverp->dtypeSetVoid(); // TODO: this is sloppy but harmless
AstCAwait* const awaitp = new AstCAwait{flp, foreverp}; AstCAwait* const awaitp = new AstCAwait{flp, foreverp};
awaitp->dtypeSetVoid(); awaitp->dtypeSetVoid();

View File

@ -6708,7 +6708,7 @@ class WidthVisitor final : public VNVisitor {
classp->baseMostClassp()->needRNG(true); classp->baseMostClassp()->needRNG(true);
v3Global.useRandomizeMethods(true); v3Global.useRandomizeMethods(true);
AstCExpr* const newp AstCExpr* const newp
= new AstCExpr{nodep->fileline(), "__Vm_rng.get_randstate()", 1, true}; = new AstCExpr{nodep->fileline(), "__Vm_rng.get_randstate()", 1};
newp->dtypeSetString(); newp->dtypeSetString();
nodep->replaceWith(newp); nodep->replaceWith(newp);
VL_DO_DANGLING(pushDeletep(nodep), nodep); VL_DO_DANGLING(pushDeletep(nodep), nodep);
@ -6721,7 +6721,7 @@ class WidthVisitor final : public VNVisitor {
classp->baseMostClassp()->needRNG(true); classp->baseMostClassp()->needRNG(true);
v3Global.useRandomizeMethods(true); v3Global.useRandomizeMethods(true);
AstCExpr* const newp AstCExpr* const newp
= new AstCExpr{nodep->fileline(), "__Vm_rng.set_randstate(", 1, true}; = new AstCExpr{nodep->fileline(), "__Vm_rng.set_randstate(", 1};
newp->addExprsp(exprp->unlinkFrBack()); newp->addExprsp(exprp->unlinkFrBack());
newp->addExprsp(new AstText{nodep->fileline(), ")", true}); newp->addExprsp(new AstText{nodep->fileline(), ")", true});
newp->dtypeSetString(); newp->dtypeSetString();