Skip to content

Commit

Permalink
astgen: Enforce the use of aliased operand accessors
Browse files Browse the repository at this point in the history
This patch enforces the use of the most specific accessors for operands
which have an '@Astgen alias' declaration, by making the superclass
accessors of the same operands private. This ensures client code is
cleaner as you can't use multiple different methods to reference the
same operands (which we used to in some places). Also prep for some
refactoring.
  • Loading branch information
gezalore committed Nov 12, 2023
1 parent 1c0af6c commit 067d40f
Show file tree
Hide file tree
Showing 13 changed files with 100 additions and 85 deletions.
4 changes: 2 additions & 2 deletions src/V3AstNodeExpr.h
Original file line number Diff line number Diff line change
Expand Up @@ -4311,7 +4311,7 @@ class AstSelMinus final : public AstNodePreSel {
// -: range extraction, perhaps with non-constant selection
// Gets replaced during link with AstSel
// @astgen alias op2 := bitp
// @astgen alias op3 := widtph
// @astgen alias op3 := widthp
public:
AstSelMinus(FileLine* fl, AstNodeExpr* fromp, AstNodeExpr* bitp, AstNodeExpr* widthp)
: ASTGEN_SUPER_SelMinus(fl, fromp, bitp, widthp) {}
Expand All @@ -4321,7 +4321,7 @@ class AstSelPlus final : public AstNodePreSel {
// +: range extraction, perhaps with non-constant selection
// Gets replaced during link with AstSel
// @astgen alias op2 := bitp
// @astgen alias op3 := widtph
// @astgen alias op3 := widthp
public:
AstSelPlus(FileLine* fl, AstNodeExpr* fromp, AstNodeExpr* bitp, AstNodeExpr* widthp)
: ASTGEN_SUPER_SelPlus(fl, fromp, bitp, widthp) {}
Expand Down
38 changes: 19 additions & 19 deletions src/V3Const.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ class ConstBitOpTreeVisitor final : public VNVisitorConst {
void visit(AstWordSel* nodep) override {
CONST_BITOP_RETURN_IF(!m_leafp, nodep);
AstConst* const constp = VN_CAST(nodep->bitp(), Const);
CONST_BITOP_RETURN_IF(!constp, nodep->rhsp());
CONST_BITOP_RETURN_IF(!constp, nodep->bitp());
UASSERT_OBJ(m_leafp->wordIdx() == -1, nodep, "Unexpected nested WordSel");
m_leafp->wordIdx(constp->toSInt());
iterateConst(nodep->fromp());
Expand Down Expand Up @@ -1269,9 +1269,9 @@ class ConstVisitor final : public VNVisitor {
// V3Expand may make a arraysel that exceeds the bounds of the array
// It was an expression, then got constified. In reality, the WordSel
// must be wrapped in a Cond, that will be false.
return (VN_IS(nodep->rhsp(), Const) && VN_IS(nodep->fromp(), NodeVarRef)
return (VN_IS(nodep->bitp(), Const) && VN_IS(nodep->fromp(), NodeVarRef)
&& VN_AS(nodep->fromp(), NodeVarRef)->access().isReadOnly()
&& (static_cast<int>(VN_AS(nodep->rhsp(), Const)->toUInt())
&& (static_cast<int>(VN_AS(nodep->bitp(), Const)->toUInt())
>= VN_AS(nodep->fromp(), NodeVarRef)->varp()->widthWords()));
}
bool operandSelFull(const AstSel* nodep) {
Expand Down Expand Up @@ -2539,7 +2539,7 @@ class ConstVisitor final : public VNVisitor {
// SEL(REPLICATE(from,rep),lsb,width) => SEL(from,0,width) as long
// as SEL's width <= b's width
AstReplicate* const repp = VN_AS(nodep->fromp(), Replicate);
AstNodeExpr* const fromp = repp->lhsp();
AstNodeExpr* const fromp = repp->srcp();
AstConst* const lsbp = VN_CAST(nodep->lsbp(), Const);
if (!lsbp) return false;
AstNodeExpr* const widthp = nodep->widthp();
Expand All @@ -2562,11 +2562,11 @@ class ConstVisitor final : public VNVisitor {
}
bool operandRepRep(AstReplicate* nodep) {
// REPLICATE(REPLICATE2(from2,cnt2),cnt1) => REPLICATE(from2,(cnt1+cnt2))
AstReplicate* const rep2p = VN_AS(nodep->lhsp(), Replicate);
AstNodeExpr* const from2p = rep2p->lhsp();
AstConst* const cnt1p = VN_CAST(nodep->rhsp(), Const);
AstReplicate* const rep2p = VN_AS(nodep->srcp(), Replicate);
AstNodeExpr* const from2p = rep2p->srcp();
AstConst* const cnt1p = VN_CAST(nodep->countp(), Const);
if (!cnt1p) return false;
AstConst* const cnt2p = VN_CAST(rep2p->rhsp(), Const);
AstConst* const cnt2p = VN_CAST(rep2p->countp(), Const);
if (!cnt2p) return false;
//
from2p->unlinkFrBack();
Expand All @@ -2589,15 +2589,15 @@ class ConstVisitor final : public VNVisitor {
AstNodeExpr* from2p = nodep->rhsp();
uint32_t cnt2 = 1;
if (VN_IS(from1p, Replicate)) {
AstConst* const cnt1p = VN_CAST(VN_CAST(from1p, Replicate)->rhsp(), Const);
AstConst* const cnt1p = VN_CAST(VN_CAST(from1p, Replicate)->countp(), Const);
if (!cnt1p) return false;
from1p = VN_AS(from1p, Replicate)->lhsp();
from1p = VN_AS(from1p, Replicate)->srcp();
cnt1 = cnt1p->toUInt();
}
if (VN_IS(from2p, Replicate)) {
AstConst* const cnt2p = VN_CAST(VN_CAST(from2p, Replicate)->rhsp(), Const);
AstConst* const cnt2p = VN_CAST(VN_CAST(from2p, Replicate)->countp(), Const);
if (!cnt2p) return false;
from2p = VN_AS(from2p, Replicate)->lhsp();
from2p = VN_AS(from2p, Replicate)->srcp();
cnt2 = cnt2p->toUInt();
}
if (!operandsSame(from1p, from2p)) return false;
Expand Down Expand Up @@ -3488,8 +3488,8 @@ class ConstVisitor final : public VNVisitor {
TREEOPA("AstNodeCond{$condp.isNeqZero, $thenp.castConst, $elsep.castConst}", "replaceWChild(nodep,$thenp)");
TREEOP ("AstNodeCond{$condp, operandsSame($thenp,,$elsep)}","replaceWChild(nodep,$thenp)");
// This visit function here must allow for short-circuiting.
TREEOPS("AstCond {$lhsp.isZero}", "replaceWIteratedThs(nodep)");
TREEOPS("AstCond {$lhsp.isNeqZero}", "replaceWIteratedRhs(nodep)");
TREEOPS("AstCond {$condp.isZero}", "replaceWIteratedThs(nodep)");
TREEOPS("AstCond {$condp.isNeqZero}", "replaceWIteratedRhs(nodep)");
TREEOP ("AstCond{$condp.castNot, $thenp, $elsep}", "AstCond{$condp->castNot()->lhsp(), $elsep, $thenp}");
TREEOP ("AstNodeCond{$condp.width1, $thenp.width1, $thenp.isAllOnes, $elsep}", "AstLogOr {$condp, $elsep}"); // a?1:b == a||b
TREEOP ("AstNodeCond{$condp.width1, $thenp.width1, $thenp, $elsep.isZero, !$elsep.isClassHandleValue}", "AstLogAnd{$condp, $thenp}"); // a?b:0 == a&&b
Expand Down Expand Up @@ -3670,9 +3670,9 @@ class ConstVisitor final : public VNVisitor {
// bits end up in the wrong places
TREEOPV("AstExtend {$lhsp.castExtend}", "replaceExtend(nodep, VN_AS(nodep->lhsp(), Extend)->lhsp())");
TREEOPV("AstExtendS{$lhsp.castExtendS}", "replaceExtend(nodep, VN_AS(nodep->lhsp(), ExtendS)->lhsp())");
TREEOPV("AstReplicate{$lhsp, $rhsp.isOne, $lhsp->width()==nodep->width()}", "replaceWLhs(nodep)"); // {1{lhs}}->lhs
TREEOPV("AstReplicate{$srcp, $countp.isOne, $srcp->width()==nodep->width()}", "replaceWLhs(nodep)"); // {1{lhs}}->lhs
TREEOPV("AstReplicateN{$lhsp, $rhsp.isOne, $lhsp->width()==nodep->width()}", "replaceWLhs(nodep)"); // {1{lhs}}->lhs
TREEOPV("AstReplicate{$lhsp.castReplicate, operandRepRep(nodep)}", "DONE"); // {2{3{lhs}}}->{6{lhs}}
TREEOPV("AstReplicate{$srcp.castReplicate, operandRepRep(nodep)}", "DONE"); // {2{3{lhs}}}->{6{lhs}}
TREEOPV("AstConcat{operandConcatSame(nodep)}", "DONE"); // {a,a}->{2{a}}, {a,2{a}}->{3{a}, etc
// Next rule because AUTOINST puts the width of bits in
// to pins, even when the widths are exactly the same across the hierarchy.
Expand All @@ -3694,9 +3694,9 @@ class ConstVisitor final : public VNVisitor {
// win if bit select is a constant (otherwise we may need to compute bit index several times)
TREEOPV("AstSel{$fromp.castBufIf1}", "replaceSelIntoBiop(nodep)");
TREEOPV("AstSel{$fromp.castNot}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castAnd,$lhsp.castConst}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castOr,$lhsp.castConst}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castXor,$lhsp.castConst}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castAnd,$fromp.castConst}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castOr,$fromp.castConst}", "replaceSelIntoUniop(nodep)");
TREEOPV("AstSel{$fromp.castXor,$fromp.castConst}", "replaceSelIntoUniop(nodep)");
// This visit function here must allow for short-circuiting.
TREEOPS("AstLogIf{$lhsp.isZero}", "replaceNum(nodep, 1)");
TREEOPV("AstLogIf{$lhsp, $rhsp}", "AstLogOr{AstLogNot{$lhsp},$rhsp}");
Expand Down
20 changes: 10 additions & 10 deletions src/V3EmitCFunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -1184,25 +1184,25 @@ class EmitCFunc VL_NOT_FINAL : public EmitCConstInit {
}
void visit(AstSel* nodep) override {
// Note ASSIGN checks for this on a LHS
emitOpName(nodep, nodep->emitC(), nodep->fromp(), nodep->lsbp(), nodep->thsp());
emitOpName(nodep, nodep->emitC(), nodep->fromp(), nodep->lsbp(), nodep->widthp());
}
void visit(AstReplicate* nodep) override {
if (nodep->lhsp()->widthMin() == 1 && !nodep->isWide()) {
UASSERT_OBJ((static_cast<int>(VN_AS(nodep->rhsp(), Const)->toUInt())
* nodep->lhsp()->widthMin())
if (nodep->srcp()->widthMin() == 1 && !nodep->isWide()) {
UASSERT_OBJ((static_cast<int>(VN_AS(nodep->countp(), Const)->toUInt())
* nodep->srcp()->widthMin())
== nodep->widthMin(),
nodep, "Replicate non-constant or width miscomputed");
puts("VL_REPLICATE_");
emitIQW(nodep);
puts("OI(");
if (nodep->lhsp()) puts(cvtToStr(nodep->lhsp()->widthMin()));
if (nodep->srcp()) puts(cvtToStr(nodep->srcp()->widthMin()));
puts(",");
iterateAndNextConstNull(nodep->lhsp());
iterateAndNextConstNull(nodep->srcp());
puts(", ");
iterateAndNextConstNull(nodep->rhsp());
iterateAndNextConstNull(nodep->countp());
puts(")");
} else {
emitOpName(nodep, nodep->emitC(), nodep->lhsp(), nodep->rhsp(), nullptr);
emitOpName(nodep, nodep->emitC(), nodep->srcp(), nodep->countp(), nullptr);
}
}
void visit(AstStreamL* nodep) override {
Expand All @@ -1229,9 +1229,9 @@ class EmitCFunc VL_NOT_FINAL : public EmitCConstInit {
}
void visit(AstCastDynamic* nodep) override {
putbs("VL_CAST_DYNAMIC(");
iterateAndNextConstNull(nodep->lhsp());
iterateAndNextConstNull(nodep->fromp());
puts(", ");
iterateAndNextConstNull(nodep->rhsp());
iterateAndNextConstNull(nodep->top());
puts(")");
}
void visit(AstCountBits* nodep) override {
Expand Down
8 changes: 4 additions & 4 deletions src/V3Expand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -731,15 +731,15 @@ class ExpandVisitor final : public VNVisitor {
} else {
if (isImpure(nodep)) return;
FileLine* const fl = nodep->fileline();
AstNodeExpr* lhsp = nodep->lhsp()->unlinkFrBack();
AstNodeExpr* lhsp = nodep->srcp()->unlinkFrBack();
AstNodeExpr* newp;
const int lhswidth = lhsp->widthMin();
if (lhswidth == 1) {
UINFO(8, " REPLICATE(w1) " << nodep << endl);
newp = new AstNegate{fl, lhsp};
} else {
UINFO(8, " REPLICATE " << nodep << endl);
const AstConst* const constp = VN_AS(nodep->rhsp(), Const);
const AstConst* const constp = VN_AS(nodep->countp(), Const);
UASSERT_OBJ(constp, nodep,
"Replication value isn't a constant. Checked earlier!");
const uint32_t times = constp->toUInt();
Expand All @@ -765,9 +765,9 @@ class ExpandVisitor final : public VNVisitor {
UINFO(8, " Wordize ASSIGN(REPLICATE) " << nodep << endl);
if (!doExpandWide(rhsp)) return false;
FileLine* const fl = nodep->fileline();
AstNodeExpr* const lhsp = rhsp->lhsp();
AstNodeExpr* const lhsp = rhsp->srcp();
const int lhswidth = lhsp->widthMin();
const AstConst* const constp = VN_AS(rhsp->rhsp(), Const);
const AstConst* const constp = VN_AS(rhsp->countp(), Const);
UASSERT_OBJ(constp, rhsp, "Replication value isn't a constant. Checked earlier!");
const uint32_t times = constp->toUInt();
for (int w = 0; w < rhsp->widthWords(); ++w) {
Expand Down
12 changes: 6 additions & 6 deletions src/V3Inst.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -362,21 +362,21 @@ class InstDeVisitor final : public VNVisitor {
} // end expanding ranged cell
else if (AstArraySel* const arrselp = VN_CAST(nodep->exprp(), ArraySel)) {
if (const AstUnpackArrayDType* const arrp
= VN_CAST(arrselp->lhsp()->dtypep(), UnpackArrayDType)) {
= VN_CAST(arrselp->fromp()->dtypep(), UnpackArrayDType)) {
if (!VN_IS(arrp->subDTypep(), IfaceRefDType)) return;
// Interface pin attaches to one element of arrayed interface
V3Const::constifyParamsEdit(arrselp->rhsp());
const AstConst* const constp = VN_CAST(arrselp->rhsp(), Const);
V3Const::constifyParamsEdit(arrselp->bitp());
const AstConst* const constp = VN_CAST(arrselp->bitp(), Const);
if (!constp) {
nodep->v3warn(
E_UNSUPPORTED,
"Unsupported: Non-constant index when passing interface to module");
return;
}
const string index = AstNode::encodeNumber(constp->toSInt());
if (VN_IS(arrselp->lhsp(), SliceSel))
arrselp->lhsp()->v3error("Unsupported: interface slices");
const AstVarRef* const varrefp = VN_CAST(arrselp->lhsp(), VarRef);
if (VN_IS(arrselp->fromp(), SliceSel))
arrselp->fromp()->v3error("Unsupported: interface slices");
const AstVarRef* const varrefp = VN_CAST(arrselp->fromp(), VarRef);
UASSERT_OBJ(varrefp, arrselp, "No interface varref under array");
AstVarXRef* const newp = new AstVarXRef{
nodep->fileline(), varrefp->name() + "__BRA__" + index + "__KET__", "",
Expand Down
10 changes: 5 additions & 5 deletions src/V3LinkLValue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,19 +238,19 @@ class LinkLValueVisitor final : public VNVisitor {
void visit(AstSel* nodep) override {
VL_RESTORER(m_setRefLvalue);
{
iterateAndNextNull(nodep->lhsp());
iterateAndNextNull(nodep->fromp());
// Only set lvalues on the from
m_setRefLvalue = VAccess::NOCHANGE;
iterateAndNextNull(nodep->rhsp());
iterateAndNextNull(nodep->thsp());
iterateAndNextNull(nodep->lsbp());
iterateAndNextNull(nodep->widthp());
}
}
void visit(AstNodeSel* nodep) override {
VL_RESTORER(m_setRefLvalue);
{ // Only set lvalues on the from
iterateAndNextNull(nodep->lhsp());
iterateAndNextNull(nodep->fromp());
m_setRefLvalue = VAccess::NOCHANGE;
iterateAndNextNull(nodep->rhsp());
iterateAndNextNull(nodep->bitp());
}
}
void visit(AstCellArrayRef* nodep) override {
Expand Down
2 changes: 1 addition & 1 deletion src/V3LinkParse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ class LinkParseVisitor final : public VNVisitor {
// Convert to AstSelLoopVars so V3LinkDot knows what's being defined
AstNode* const newp
= new AstSelLoopVars{selp->fileline(), selp->fromp()->unlinkFrBack(),
selp->rhsp()->unlinkFrBackWithNext()};
selp->bitp()->unlinkFrBackWithNext()};
selp->replaceWith(newp);
VL_DO_DANGLING(selp->deleteTree(), selp);
} else if (VN_IS(bracketp, SelLoopVars)) {
Expand Down
2 changes: 1 addition & 1 deletion src/V3Simulate.h
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ class SimulateVisitor VL_NOT_FINAL : public VNVisitorConst {
outVarrefpRef = varrefp;
lsbRef = fetchConst(selp->lsbp())->num();
return; // And presumably still optimizable()
} else if (AstSel* const subselp = VN_CAST(selp->lhsp(), Sel)) {
} else if (AstSel* const subselp = VN_CAST(selp->fromp(), Sel)) {
V3Number sublsb{nodep};
handleAssignSelRecurse(nodep, subselp, outVarrefpRef, sublsb /*ref*/, depth + 1);
if (optimizable()) {
Expand Down
2 changes: 1 addition & 1 deletion src/V3Slice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ class SliceVisitor final : public VNVisitor {
if (AstSliceSel* const slicep = VN_CAST(itemp, SliceSel)) {
offset += slicep->declRange().lo();
newp = new AstArraySel{nodep->fileline(),
slicep->lhsp()->cloneTreePure(false), offset};
slicep->fromp()->cloneTreePure(false), offset};
} else {
newp = new AstArraySel{nodep->fileline(), itemp->cloneTreePure(false),
offset};
Expand Down
14 changes: 7 additions & 7 deletions src/V3Subst.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ class SubstVisitor final : public VNVisitor {
}
}
} else if (const AstWordSel* const wordp = VN_CAST(nodep->lhsp(), WordSel)) {
if (AstVarRef* const varrefp = VN_CAST(wordp->lhsp(), VarRef)) {
if (VN_IS(wordp->rhsp(), Const) && isSubstVar(varrefp->varp())) {
const int word = VN_AS(wordp->rhsp(), Const)->toUInt();
if (AstVarRef* const varrefp = VN_CAST(wordp->fromp(), VarRef)) {
if (VN_IS(wordp->bitp(), Const) && isSubstVar(varrefp->varp())) {
const int word = VN_AS(wordp->bitp(), Const)->toUInt();
SubstVarEntry* const entryp = getEntryp(varrefp);
hit = true;
if (m_ops > SUBST_MAX_OPS_SUBST) {
Expand Down Expand Up @@ -297,9 +297,9 @@ class SubstVisitor final : public VNVisitor {
}
void visit(AstWordSel* nodep) override {
if (!m_funcp) return;
iterate(nodep->rhsp());
AstVarRef* const varrefp = VN_CAST(nodep->lhsp(), VarRef);
const AstConst* const constp = VN_CAST(nodep->rhsp(), Const);
iterate(nodep->bitp());
AstVarRef* const varrefp = VN_CAST(nodep->fromp(), VarRef);
const AstConst* const constp = VN_CAST(nodep->bitp(), Const);
if (varrefp && isSubstVar(varrefp->varp()) && varrefp->access().isReadOnly() && constp) {
// Nicely formed lvalues handled in NodeAssign
// Other lvalues handled as unknown mess in AstVarRef
Expand All @@ -318,7 +318,7 @@ class SubstVisitor final : public VNVisitor {
entryp->consumeWord(word);
}
} else {
iterate(nodep->lhsp());
iterate(nodep->fromp());
}
}
void visit(AstVarRef* nodep) override {
Expand Down
Loading

0 comments on commit 067d40f

Please sign in to comment.