Skip to content

Commit

Permalink
Support resizing function call inout arguments (verilator#4467).
Browse files Browse the repository at this point in the history
  • Loading branch information
wsnyder committed Sep 17, 2023
1 parent f507ee5 commit ab13548
Show file tree
Hide file tree
Showing 15 changed files with 408 additions and 134 deletions.
1 change: 1 addition & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Verilator 5.017 devel

**Minor:**

* Support resizing function call inout arguments (#4467).


Verilator 5.016 2023-09-16
Expand Down
15 changes: 15 additions & 0 deletions src/V3AstNodeExpr.h
Original file line number Diff line number Diff line change
Expand Up @@ -5083,6 +5083,21 @@ class AstRedXor final : public AstNodeUniop {
bool sizeMattersLhs() const override { return false; }
int instrCount() const override { return 1 + V3Number::log2b(width()); }
};
class AstResizeLValue final : public AstNodeUniop {
// Resize a LValue into a wider/narrower entity at function argument boundry
// Width and signness is implied from nodep->width()
public:
AstResizeLValue(FileLine* fl, AstNodeExpr* lhsp)
: ASTGEN_SUPER_ResizeLValue(fl, lhsp) {}
ASTGEN_MEMBERS_AstResizeLValue;
void numberOperate(V3Number& out, const V3Number& lhs) override { V3ERROR_NA; }
string emitVerilog() override { return "%l"; }
string emitC() final override { V3ERROR_NA_RETURN(""); }
bool cleanOut() const override { return true; }
bool cleanLhs() const override { return true; }
bool sizeMattersLhs() const override { return false; }
int instrCount() const override { return 0; }
};
class AstSigned final : public AstNodeUniop {
// $signed(lhs)
public:
Expand Down
105 changes: 76 additions & 29 deletions src/V3Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,53 @@ class TaskVisitor final : public VNVisitor {
});
}

AstAssign* connectPortMakeInAssign(AstNodeExpr* pinp, AstVarScope* newvscp, bool pureCheck) {
// Create input assignment to go in FRONT of function call
AstNodeExpr* inPinp = pinp;
if (AstResizeLValue* sinPinp = VN_CAST(inPinp, ResizeLValue)) inPinp = sinPinp->lhsp();
AstNodeExpr* const inPinClonep
= pureCheck ? inPinp->cloneTreePure(true) : inPinp->cloneTree(true);
AstAssign* const assp = new AstAssign{
pinp->fileline(), new AstVarRef{newvscp->fileline(), newvscp, VAccess::WRITE},
inPinClonep};
assp->fileline()->modifyWarnOff(V3ErrorCode::BLKSEQ, true); // Ok if in <= block
return assp;
}
AstAssign* connectPortMakeOutAssign(AstVar* portp, AstNodeExpr* pinp, AstVarScope* newvscp,
bool pureCheck) {
// If needed, remap size of function to caller's output size
AstNodeExpr* outPinp = pinp;
AstNodeExpr* postRhsp = new AstVarRef{newvscp->fileline(), newvscp, VAccess::READ};
if (AstResizeLValue* soutPinp = VN_CAST(outPinp, ResizeLValue)) {
outPinp = soutPinp->lhsp();
if (AstNodeUniop* soutPinp = VN_CAST(outPinp, Extend)) {
outPinp = soutPinp->lhsp();
} else if (AstNodeUniop* soutPinp = VN_CAST(outPinp, ExtendS)) {
outPinp = soutPinp->lhsp();
} else if (AstSel* soutPinp = VN_CAST(outPinp, Sel)) {
outPinp = soutPinp->fromp();
} else {
outPinp->v3fatalSrc("Inout pin resizing should have had extend or select");
}
if (outPinp->width() < portp->width()) {
postRhsp = new AstSel{pinp->fileline(), postRhsp, 0, pinp->width()};
} else { // pin width > port width
if (pinp->isSigned() && postRhsp->isSigned()) {
postRhsp = new AstExtendS{pinp->fileline(), postRhsp};
} else {
postRhsp = new AstExtend{pinp->fileline(), postRhsp};
}
}
postRhsp->dtypeFrom(outPinp);
}
// Put output assignment AFTER function call
AstNodeExpr* const outPinClonep
= pureCheck ? outPinp->cloneTreePure(true) : outPinp->cloneTree(true);
AstAssign* const assp = new AstAssign{pinp->fileline(), outPinClonep, postRhsp};
assp->fileline()->modifyWarnOff(V3ErrorCode::BLKSEQ, true); // Ok if in <= block
return assp;
}

void connectPort(AstVar* portp, AstArg* argp, const string& namePrefix, AstNode* beginp,
bool inlineTask) {
AstNodeExpr* const pinp = argp->exprp();
Expand All @@ -432,23 +479,31 @@ class TaskVisitor final : public VNVisitor {
pinp->v3error("Function/task " + portp->direction().prettyName() // e.g. "output"
+ " connected to constant instead of variable: "
+ portp->prettyNameQ());
} else if (portp->isInoutish()) {
// Correct lvalue; see comments below
}
// else if (portp->direction() == VDirection::REF) {
// TODO References need to instead pass a real reference var, see issue #3385
else if (portp->isInoutish()) {
// if (debug() >= 9) pinp->dumpTree("-pinrsize- ");
V3LinkLValue::linkLValueSet(pinp);

if (AstVarRef* const varrefp = VN_CAST(pinp, VarRef)) {
// Connect to this exact variable
if (inlineTask) {
AstVarScope* const localVscp = varrefp->varScopep();
UASSERT_OBJ(localVscp, varrefp, "Null var scope");
portp->user2p(localVscp);
pushDeletep(pinp);
}
} else {
pinp->v3warn(
E_TASKNSVAR,
"Unsupported: Function/task input argument is not simple variable");
AstVarScope* const newvscp
= createVarScope(portp, namePrefix + "__" + portp->shortName());
portp->user2p(newvscp);
if (!inlineTask)
pinp->replaceWith(
new AstVarRef{newvscp->fileline(), newvscp, VAccess::READWRITE});

// Put input assignment in FRONT of all other statements
AstAssign* const preassp = connectPortMakeInAssign(pinp, newvscp, true);
if (AstNode* const afterp = beginp->nextp()) {
afterp->unlinkFrBackWithNext();
AstNode::addNext<AstNode, AstNode>(preassp, afterp);
}
beginp->addNext(preassp);

AstAssign* const postassp = connectPortMakeOutAssign(portp, pinp, newvscp, true);
beginp->addNext(postassp);
// if (debug() >= 9) beginp->dumpTreeAndNext(cout, "-pinrsize-out- ");
} else if (portp->isWritable()) {
// Make output variables
// Correct lvalue; we didn't know when we linked
Expand All @@ -463,29 +518,21 @@ class TaskVisitor final : public VNVisitor {
portp->user2p(newvscp);
if (!inlineTask)
pinp->replaceWith(new AstVarRef{newvscp->fileline(), newvscp, VAccess::WRITE});
AstAssign* const assp
= new AstAssign{pinp->fileline(), pinp,
new AstVarRef{newvscp->fileline(), newvscp, VAccess::READ}};
assp->fileline()->modifyWarnOff(V3ErrorCode::BLKSEQ,
true); // Ok if in <= block
AstAssign* const postassp = connectPortMakeOutAssign(portp, pinp, newvscp, false);
// Put assignment BEHIND of all other statements
beginp->addNext(assp);
beginp->addNext(postassp);
} else if (inlineTask && portp->isNonOutput()) {
// Make input variable
AstVarScope* const inVscp
AstVarScope* const newvscp
= createVarScope(portp, namePrefix + "__" + portp->shortName());
portp->user2p(inVscp);
AstAssign* const assp = new AstAssign{
pinp->fileline(), new AstVarRef{inVscp->fileline(), inVscp, VAccess::WRITE},
pinp};
assp->fileline()->modifyWarnOff(V3ErrorCode::BLKSEQ,
true); // Ok if in <= block
portp->user2p(newvscp);
AstAssign* const preassp = connectPortMakeInAssign(pinp, newvscp, false);
// Put assignment in FRONT of all other statements
if (AstNode* const afterp = beginp->nextp()) {
afterp->unlinkFrBackWithNext();
AstNode::addNext<AstNode, AstNode>(assp, afterp);
AstNode::addNext<AstNode, AstNode>(preassp, afterp);
}
beginp->addNext(assp);
beginp->addNext(preassp);
}
}
}
Expand Down
18 changes: 10 additions & 8 deletions src/V3Width.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5577,7 +5577,7 @@ class WidthVisitor final : public VNVisitor {
for (const auto& tconnect : tconnects) {
const AstVar* const portp = tconnect.first;
const AstArg* const argp = tconnect.second;
AstNode* const pinp = argp->exprp();
AstNodeExpr* const pinp = argp->exprp();
if (!pinp) continue; // Argument error we'll find later
AstNodeDType* const portDTypep = portp->dtypep()->skipRefToEnump();
const AstNodeDType* const pinDTypep = pinp->dtypep()->skipRefToEnump();
Expand All @@ -5588,13 +5588,15 @@ class WidthVisitor final : public VNVisitor {
<< portDTypep->prettyDTypeName() << " but connection is "
<< pinDTypep->prettyDTypeName() << ".");
} else if (portp->isWritable() && pinp->width() != portp->width()) {
pinp->v3warn(E_UNSUPPORTED, "Unsupported: Function output argument "
<< portp->prettyNameQ() << " requires "
<< portp->width() << " bits, but connection's "
<< pinp->prettyTypeName() << " generates "
<< pinp->width() << " bits.");
// otherwise would need some mess to force both sides to proper size
// (get an ASSIGN with EXTEND on the lhs instead of rhs)
pinp->v3widthWarn(portp->width(), pinp->width(),
"Function output argument "
<< portp->prettyNameQ() << " requires " << portp->width()
<< " bits, but connection's " << pinp->prettyTypeName()
<< " generates " << pinp->width() << " bits.");
VNRelinker relinkHandle;
pinp->unlinkFrBack(&relinkHandle);
AstNodeExpr* const newp = new AstResizeLValue{pinp->fileline(), pinp};
relinkHandle.relink(newp);
}
if (!portp->basicp() || portp->basicp()->isOpaque()) {
checkClassAssign(nodep, "Function Argument", pinp, portDTypep);
Expand Down
7 changes: 4 additions & 3 deletions test_regress/t/t_func_bad.out
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@
: ... In instance t
11 | x;
| ^
%Error-UNSUPPORTED: t/t_func_bad.v:11:7: Unsupported: Function output argument 'y' requires 1 bits, but connection's CONST '?32?h0' generates 32 bits.
: ... In instance t
%Warning-WIDTHTRUNC: t/t_func_bad.v:11:7: Function output argument 'y' requires 1 bits, but connection's CONST '?32?h0' generates 32 bits.
: ... In instance t
11 | x;
| ^
... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
... For warning description see https://verilator.org/warn/WIDTHTRUNC?v=latest
... Use "/* verilator lint_off WIDTHTRUNC */" and lint_on around source to disable this message.
%Error: t/t_func_bad.v:14:17: No such argument 'no_such' in function call to FUNC 'f'
: ... In instance t
14 | f(.j(1), .no_such(2));
Expand Down
8 changes: 0 additions & 8 deletions test_regress/t/t_func_tasknsvar_bad.out

This file was deleted.

19 changes: 0 additions & 19 deletions test_regress/t/t_func_tasknsvar_bad.v

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@
# Version 2.0.
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0

scenarios(vlt => 1);
scenarios(simulator => 1);

lint(
fails => 1,
expect_filename => $Self->{golden_filename},
compile(
verilator_flags2 => ["-Wno-WIDTHTRUNC"],
v_flags2 => ["+define+T_FUNC_WIDE_OUT t/t_func_wide_out_c.cpp"],
);

execute(
check_finished => 1,
);

ok(1);
Expand Down
125 changes: 125 additions & 0 deletions test_regress/t/t_func_wide_out.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2023 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0

`define stop $stop
`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);

typedef bit signed [11:0] s12_t;
typedef bit unsigned [11:0] u12_t;
typedef bit signed [69:0] s70_t;
typedef bit unsigned [69:0] u70_t;

import "DPI-C" context function void dpii_inv_s12(input s12_t in, output s12_t out);
import "DPI-C" context function void dpii_inv_u12(input u12_t in, output u12_t out);
import "DPI-C" context function void dpii_inv_s70(input s70_t in, output s70_t out);
import "DPI-C" context function void dpii_inv_u70(input s70_t in, output u70_t out);

class Cls #(type T = bit);
static function void get(inout T value);
`ifdef TEST_NOINLINE
// verilator no_inline_task
`endif
value = ~value;
endfunction
endclass

module t;

parameter MSG_PORT_WIDTH = 4350;
localparam PAYLOAD_MAX_BITS = 4352;

reg [MSG_PORT_WIDTH-1:0] msg;

function void func (output bit [PAYLOAD_MAX_BITS-1:0] data);
`ifdef TEST_NOINLINE
// verilator no_inline_task
`endif
data = {PAYLOAD_MAX_BITS{1'b1}};
endfunction

s12_t ds12;
u12_t du12;
s70_t ds70;
u70_t du70;
s12_t qs12;
u12_t qu12;
s70_t qs70;
u70_t qu70;

initial begin
// Operator TASKREF 'func' expects 4352 bits on the Function Argument, but Function Argument's VARREF 'msg' generates 4350 bits.
// verilator lint_off WIDTHEXPAND
func(msg);
if (msg !== {MSG_PORT_WIDTH{1'b1}}) $stop;

begin
// narrow connect to wide
ds12 = 12'h234;
Cls#(s70_t)::get(ds12);
`checkh(ds12, 12'hdcb);
ds12 = 12'he34; // negative if signed
Cls#(s70_t)::get(ds12);
`checkh(ds12, 12'h1cb);

du12 = 12'h244;
Cls#(u70_t)::get(du12);
`checkh(du12, 12'hdbb);
du12 = 12'he34; // negative if signed
Cls#(u70_t)::get(du12);
`checkh(du12, 12'h1cb);

// wie connect to narrow
ds70 = 12'h254;
Cls#(s12_t)::get(ds70);
`checkh(ds70, 70'h3ffffffffffffffdab);
ds70 = 12'he34; // negative if signed
Cls#(s12_t)::get(ds70);
`checkh(ds70, 70'h0000000000000001cb);

du70 = 12'h264;
Cls#(u12_t)::get(du70);
`checkh(du70, 70'h000000000000000d9b);
du70 = 12'he34; // negative if signed
Cls#(u12_t)::get(du70);
`checkh(du70, 70'h0000000000000001cb);
end

begin
// narrow connect to wide
ds12 = 12'h234;
dpii_inv_s70(ds12, qs12);
`checkh(qs12, 12'hdcb);
ds12 = 12'he34; // negative if signed
dpii_inv_s70(ds12, qs12);
`checkh(qs12, 12'h1cb);

du12 = 12'h244;
dpii_inv_u70(du12, qu12);
`checkh(qu12, 12'hdbb);
du12 = 12'he34; // negative if signed
dpii_inv_u70(ds12, qs12);
`checkh(qs12, 12'h1cb);

// wie connect to narrow
ds70 = 12'h254;
dpii_inv_s12(ds70, qs70);
`checkh(qs70, 70'h3ffffffffffffffdab);
ds70 = 12'he34; // negative if signed
dpii_inv_s12(ds70, qs70);
`checkh(qs70, 70'h0000000000000001cb);

du70 = 12'h264;
dpii_inv_u12(du70, qu70);
`checkh(qu70, 70'h000000000000000d9b);
du70 = 12'he34; // negative if signed
dpii_inv_u12(du70, qu70);
`checkh(qu70, 70'h0000000000000001cb);
end

$write("*-* All Finished *-*\n");
$finish;
end
endmodule
Loading

0 comments on commit ab13548

Please sign in to comment.