luke
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix type check for bitwise operators on unpacked arrays (#5664) 

Reject non-packed array operands (unpacked, dynamic, queue, associative)
in bitwise AND, OR, and XOR at the type-checking stage (V3Width) instead
of letting them reach V3DfgPeephole where they cause an internal error.

Add AstNodeDType::isNonPackedArray() helper and replace all existing
VN_IS(*, UnpackArrayDType) || VN_IS(*, DynArrayDType) || ... patterns
across the codebase to use it.

Closes #5664
This commit is contained in:
Eunseo Song 2026-03-27 12:13:26 +09:00
parent 5d2d05236e
commit 1946df8bc8
10 changed files with 88 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/V3AstNodeDType.h
View File

@ -125,6 +125,8 @@ public:
virtual AstNodeDType* subDTypep() const VL_MT_STABLE { return nullptr; }
virtual AstNodeDType* subDType2p() const VL_MT_STABLE { return nullptr; }
virtual bool isAggregateType() const { return false; }
// True for unpacked, dynamic, queue, and associative arrays (not packed arrays)
bool isNonPackedArray() const;
virtual bool isFourstate() const;
// Ideally an IEEE $typename
virtual string prettyDTypeName(bool) const { return prettyTypeName(); }

5
src/V3AstNodes.cpp
View File

@ -1013,6 +1013,11 @@ bool AstNodeDType::similarDType(const AstNodeDType* samep) const {
bool AstNodeDType::isFourstate() const { return basicp() && basicp()->isFourstate(); }
bool AstNodeDType::isNonPackedArray() const {
return VN_IS(this, UnpackArrayDType) || VN_IS(this, DynArrayDType)
|| VN_IS(this, QueueDType) || VN_IS(this, AssocArrayDType);
}
class AstNodeDType::CTypeRecursed final {
public:
string m_type; // The base type, e.g.: "Foo_t"s

6
src/V3Const.cpp
View File

@ -2397,8 +2397,7 @@ class ConstVisitor final : public VNVisitor {
const AstNodeDType* const srcDTypep = srcp->dtypep()->skipRefp();
// Handle unpacked/queue/dynarray source -> queue/dynarray dest via
// CvtArrayToArray (StreamL reverses, so reverse=true)
if ((VN_IS(srcDTypep, UnpackArrayDType) || VN_IS(srcDTypep, QueueDType)
|| VN_IS(srcDTypep, DynArrayDType))
if (srcDTypep->isNonPackedArray()
&& (VN_IS(dstDTypep, QueueDType) || VN_IS(dstDTypep, DynArrayDType))) {
int blockSize = 1;
if (const AstConst* const constp
@ -2461,8 +2460,7 @@ class ConstVisitor final : public VNVisitor {
origSrcp = cvtp->fromp();
}
const AstNodeDType* const origSrcDTypep = origSrcp->dtypep()->skipRefp();
if (VN_IS(origSrcDTypep, UnpackArrayDType) || VN_IS(origSrcDTypep, QueueDType)
|| VN_IS(origSrcDTypep, DynArrayDType)) {
if (origSrcDTypep->isNonPackedArray()) {
int srcElementBits = 0;
if (const AstNodeDType* const elemDtp = origSrcDTypep->subDTypep()) {
srcElementBits = elemDtp->width();

4
src/V3EmitCHeaders.cpp
View File

@ -280,9 +280,7 @@ class EmitCHeader final : public EmitCConstInit {
enum class AttributeType { Width, Dimension };
// Get member attribute based on type
int getNodeAttribute(const AstMemberDType* itemp, AttributeType type) {
const bool isArrayType
= VN_IS(itemp->dtypep(), UnpackArrayDType) || VN_IS(itemp->dtypep(), DynArrayDType)
|| VN_IS(itemp->dtypep(), QueueDType) || VN_IS(itemp->dtypep(), AssocArrayDType);
const bool isArrayType = itemp->dtypep()->isNonPackedArray();
switch (type) {
case AttributeType::Width: {
if (isArrayType) {

20
src/V3Randomize.cpp
View File

@ -1136,8 +1136,7 @@ class ConstraintExprVisitor final : public VNVisitor {
AstClassRefDType* elemClassRefDtp = nullptr;
{
AstNodeDType* varDtp = varp->dtypep()->skipRefp();
if (VN_IS(varDtp, DynArrayDType) || VN_IS(varDtp, QueueDType)
|| VN_IS(varDtp, UnpackArrayDType) || VN_IS(varDtp, AssocArrayDType)) {
if (varDtp->isNonPackedArray()) {
AstNodeDType* const elemDtp = varDtp->subDTypep()->skipRefp();
elemClassRefDtp = VN_CAST(elemDtp, ClassRefDType);
if (elemClassRefDtp) {
@ -1209,9 +1208,7 @@ class ConstraintExprVisitor final : public VNVisitor {
AstVar* const memberVarp = VN_CAST(mnodep, Var);
if (!memberVarp || !memberVarp->rand().isRandomizable()) continue;
AstNodeDType* const memberDtp = memberVarp->dtypep()->skipRefp();
if (VN_IS(memberDtp, ClassRefDType) || VN_IS(memberDtp, DynArrayDType)
|| VN_IS(memberDtp, QueueDType) || VN_IS(memberDtp, UnpackArrayDType)
|| VN_IS(memberDtp, AssocArrayDType))
if (VN_IS(memberDtp, ClassRefDType) || memberDtp->isNonPackedArray())
continue;
const int memberWidth = memberDtp->width();
@ -1281,9 +1278,7 @@ class ConstraintExprVisitor final : public VNVisitor {
VAccess::READWRITE},
VCMethod::RANDOMIZER_WRITE_VAR};
uint32_t dimension = 0;
if (VN_IS(varp->dtypep(), UnpackArrayDType) || VN_IS(varp->dtypep(), DynArrayDType)
|| VN_IS(varp->dtypep(), QueueDType)
|| VN_IS(varp->dtypep(), AssocArrayDType)) {
if (varp->dtypep()->isNonPackedArray()) {
const std::pair<uint32_t, uint32_t> dims
= varp->dtypep()->dimensions(/*includeBasic=*/true);
const uint32_t unpackedDimensions = dims.second;
@ -1316,8 +1311,7 @@ class ConstraintExprVisitor final : public VNVisitor {
methodp->addPinsp(varRefp);
}
AstNodeDType* tmpDtypep = varp->dtypep();
while (VN_IS(tmpDtypep, UnpackArrayDType) || VN_IS(tmpDtypep, DynArrayDType)
|| VN_IS(tmpDtypep, QueueDType) || VN_IS(tmpDtypep, AssocArrayDType))
while (tmpDtypep->isNonPackedArray())
tmpDtypep = tmpDtypep->subDTypep();
const size_t width = tmpDtypep->width();
methodp->addPinsp(
@ -3369,8 +3363,7 @@ class RandomizeVisitor final : public VNVisitor {
dtypep->findBasicDType(VBasicDTypeKwd::UINT32)};
};
AstNodeExpr* tempElementp = nullptr;
while (VN_IS(tempDTypep, DynArrayDType) || VN_IS(tempDTypep, UnpackArrayDType)
|| VN_IS(tempDTypep, AssocArrayDType) || VN_IS(tempDTypep, QueueDType)) {
while (tempDTypep->isNonPackedArray()) {
AstVar* const newRandLoopIndxp = createLoopIndex(tempDTypep);
randLoopIndxp = AstNode::addNext(randLoopIndxp, newRandLoopIndxp);
AstNodeExpr* const tempExprp = tempElementp ? tempElementp : exprp;
@ -4396,8 +4389,7 @@ class RandomizeVisitor final : public VNVisitor {
}
AstNodeDType* tmpDtypep = arrVarp->dtypep();
while (VN_IS(tmpDtypep, UnpackArrayDType) || VN_IS(tmpDtypep, DynArrayDType)
|| VN_IS(tmpDtypep, QueueDType) || VN_IS(tmpDtypep, AssocArrayDType))
while (tmpDtypep->isNonPackedArray())
tmpDtypep = tmpDtypep->subDTypep();
const size_t width = tmpDtypep->width();

25
src/V3Width.cpp
View File

@ -3328,8 +3328,7 @@ class WidthVisitor final : public VNVisitor {
// Similar logic in V3Case
return irangep->newAndFromInside(exprp, irangep->lhsp()->unlinkFrBack(),
irangep->rhsp()->unlinkFrBack());
} else if (VN_IS(itemDtp, UnpackArrayDType) || VN_IS(itemDtp, DynArrayDType)
|| VN_IS(itemDtp, QueueDType)) {
} else if (itemDtp->isNonPackedArray()) {
// Unsupported in parameters
AstNodeExpr* const inewp = new AstCMethodHard{nodep->fileline(), itemp->unlinkFrBack(),
VCMethod::ARRAY_INSIDE, exprp};
@ -5975,12 +5974,8 @@ class WidthVisitor final : public VNVisitor {
const AstNodeDType* const rhsDtp = nodep->rhsp()->dtypep()->skipRefp();
// Only check if number of states match for unpacked array to unpacked array
// assignments
const bool lhsIsUnpackArray
= VN_IS(lhsDtp, UnpackArrayDType) || VN_IS(lhsDtp, DynArrayDType)
|| VN_IS(lhsDtp, QueueDType) || VN_IS(lhsDtp, AssocArrayDType);
const bool rhsIsUnpackArray
= VN_IS(rhsDtp, UnpackArrayDType) || VN_IS(rhsDtp, DynArrayDType)
|| VN_IS(rhsDtp, QueueDType) || VN_IS(rhsDtp, AssocArrayDType);
const bool lhsIsUnpackArray = lhsDtp->isNonPackedArray();
const bool rhsIsUnpackArray = rhsDtp->isNonPackedArray();
if (lhsIsUnpackArray && rhsIsUnpackArray) {
if (lhsDtp->isFourstate() != rhsDtp->isFourstate()) {
nodep->v3error("Assignment between 2-state and 4-state types requires "
@ -7918,6 +7913,20 @@ class WidthVisitor final : public VNVisitor {
// Determine expression widths only relying on what's in the subops
userIterateAndNext(nodep->lhsp(), WidthVP{CONTEXT_DET, PRELIM}.p());
userIterateAndNext(nodep->rhsp(), WidthVP{CONTEXT_DET, PRELIM}.p());
// Bitwise operators are not defined for non-packed arrays
{
const AstNodeDType* const lhsDtp = nodep->lhsp()->dtypep()->skipRefp();
const AstNodeDType* const rhsDtp = nodep->rhsp()->dtypep()->skipRefp();
if (lhsDtp->isNonPackedArray() || rhsDtp->isNonPackedArray()) {
const AstNodeDType* const badDtp
= lhsDtp->isNonPackedArray() ? lhsDtp : rhsDtp;
nodep->v3error("Operator " << nodep->prettyTypeName()
<< " not defined for "
<< badDtp->prettyDTypeName(false) << " operands");
nodep->dtypeSetBit();
return;
}
}
checkCvtUS(nodep->lhsp(), false);
checkCvtUS(nodep->rhsp(), false);
const int width = std::max(nodep->lhsp()->width(), nodep->rhsp()->width());

10
test_regress/t/t_bitwise_array_bad.out Normal file
View File

@ -0,0 +1,10 @@
%Error: t/t_bitwise_array_bad.v:13:19: Operator OR not defined for logic[1:0]$[1:0] operands
: ... note: In instance 't'
13 | assign o = arr | 1;
| ^
... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
%Error: t/t_bitwise_array_bad.v:14:20: Operator OR not defined for logic[1:0]$[1:0] operands
: ... note: In instance 't'
14 | assign o2 = arr | i + 1;
| ^
%Error: Exiting due to

16
test_regress/t/t_bitwise_array_bad.py Normal file
View File

@ -0,0 +1,16 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# This program is free software; you can redistribute it and/or modify it
# under the terms of either the GNU Lesser General Public License Version 3
# or the Perl Artistic License Version 2.0.
# SPDX-FileCopyrightText: 2026 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('linter')
test.lint(fails=True, expect_filename=test.golden_filename)
test.passes()

15
test_regress/t/t_bitwise_array_bad.v Normal file
View File

@ -0,0 +1,15 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2024 Antmicro
// SPDX-License-Identifier: CC0-1.0
module t(
input [1:0] i,
output [1:0] o,
output [1:0] o2
);
wire [1:0] arr [1:0];
assign o = arr | 1;
assign o2 = arr | i + 1;
endmodule

17
test_regress/t/t_mem_slice_dtype_bad.out
View File

@ -1,5 +1,16 @@
%Error: t/t_mem_slice_dtype_bad.v:22:43: ADD unexpected in assignment to unpacked array
22 | completed_cnt[id] <= completed_cnt_dp + 1;
| ^
%Error: t/t_mem_slice_dtype_bad.v:37:25: Operator AND not defined for logic[39:0]$[1:0] operands
: ... note: In instance 't'
37 | assert ((addr[id] & way_mask) == 0);
| ^
... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance.
%Warning-WIDTHTRUNC: t/t_mem_slice_dtype_bad.v:37:25: Operator AND expects 32 or 1 bits on the LHS, but LHS's ARRAYSEL generates 40 bits.
: ... note: In instance 't'
37 | assert ((addr[id] & way_mask) == 0);
| ^
... 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.
%Warning-WIDTHTRUNC: t/t_mem_slice_dtype_bad.v:37:25: Operator AND expects 32 or 1 bits on the RHS, but RHS's VARREF 'way_mask' generates 40 bits.
: ... note: In instance 't'
37 | assert ((addr[id] & way_mask) == 0);
| ^
%Error: Exiting due to