Skip to content

Commit

Permalink
Fix dtype of condition operation on class objects (verilator#4345) (v…
Browse files Browse the repository at this point in the history
  • Loading branch information
RRozak authored Aug 7, 2023
1 parent b0942ed commit 2d9bc73
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 41 deletions.
10 changes: 2 additions & 8 deletions src/V3AstNodeExpr.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,14 +360,8 @@ class AstNodeCond VL_NOT_FINAL : public AstNodeTriop {
// @astgen alias op2 := thenp
// @astgen alias op3 := elsep
protected:
AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp, AstNodeExpr* elsep)
: AstNodeTriop{t, fl, condp, thenp, elsep} {
if (thenp) {
dtypeFrom(thenp);
} else if (elsep) {
dtypeFrom(elsep);
}
}
AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp,
AstNodeExpr* elsep);

public:
ASTGEN_MEMBERS_AstNodeCond;
Expand Down
15 changes: 15 additions & 0 deletions src/V3AstNodes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "V3Hasher.h"
#include "V3PartitionGraph.h" // Just for mtask dumping
#include "V3String.h"
#include "V3Width.h"

#include "V3Ast__gen_macros.h" // Generated by 'astgen'

Expand Down Expand Up @@ -134,6 +135,20 @@ string AstCCall::selfPointerProtect(bool useSelfForThis) const {
return VIdProtect::protectWordsIf(sp, protect());
}

AstNodeCond::AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp,
AstNodeExpr* elsep)
: AstNodeTriop{t, fl, condp, thenp, elsep} {
UASSERT_OBJ(thenp, this, "No thenp expression");
UASSERT_OBJ(elsep, this, "No elsep expression");
if (thenp->isClassHandleValue() && elsep->isClassHandleValue()) {
// Get the most-deriving class type that both arguments can be casted to.
AstNodeDType* const commonClassTypep = V3Width::getCommonClassTypep(thenp, elsep);
UASSERT_OBJ(commonClassTypep, this, "No common base class exists");
dtypep(commonClassTypep);
} else {
dtypeFrom(thenp);
}
}
void AstNodeCond::numberOperate(V3Number& out, const V3Number& lhs, const V3Number& rhs,
const V3Number& ths) {
if (lhs.isNeqZero()) {
Expand Down
71 changes: 38 additions & 33 deletions src/V3Width.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,8 @@ class WidthVisitor final : public VNVisitor {
AstNodeDType* commonClassTypep = nullptr;
if (nodep->thenp()->isClassHandleValue() && nodep->elsep()->isClassHandleValue()) {
// Get the most-deriving class type that both arguments can be casted to.
commonClassTypep = getCommonClassTypep(nodep->thenp(), nodep->elsep());
commonClassTypep
= V3Width::getCommonClassTypep(nodep->thenp(), nodep->elsep());
}
if (commonClassTypep) {
nodep->dtypep(commonClassTypep);
Expand Down Expand Up @@ -7337,41 +7338,9 @@ class WidthVisitor final : public VNVisitor {
if (nodep->subDTypep()) return hasOpenArrayIterateDType(nodep->subDTypep()->skipRefp());
return false;
}
AstNodeDType* getCommonClassTypep(AstNode* nodep1, AstNode* nodep2) {
// Return the class type that both nodep1 and nodep2 are castable to.
// If both are null, return the type of null constant.
// If one is a class and one is null, return AstClassRefDType that points to that class.
// If no common class type exists, return nullptr.

// First handle cases with null values and when one class is a super class of the other.
if (VN_IS(nodep1, Const)) std::swap(nodep1, nodep2);
const Castable castable = computeCastable(nodep1->dtypep(), nodep2->dtypep(), nodep2);
if (castable == SAMEISH || castable == COMPATIBLE) {
return nodep1->dtypep()->cloneTree(false);
} else if (castable == DYNAMIC_CLASS) {
return nodep2->dtypep()->cloneTree(false);
}

AstClassRefDType* classDtypep1 = VN_CAST(nodep1->dtypep(), ClassRefDType);
while (classDtypep1) {
const Castable castable = computeCastable(classDtypep1, nodep2->dtypep(), nodep2);
if (castable == COMPATIBLE) return classDtypep1->cloneTree(false);
AstClassExtends* const extendsp = classDtypep1->classp()->extendsp();
classDtypep1 = extendsp ? VN_AS(extendsp->dtypep(), ClassRefDType) : nullptr;
}
return nullptr;
}

//----------------------------------------------------------------------
// METHODS - casting
static Castable computeCastable(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
const AstNode* fromConstp) {
const auto castable = computeCastableImp(toDtp, fromDtp, fromConstp);
UINFO(9, " castable=" << castable << " for " << toDtp << endl);
UINFO(9, " =?= " << fromDtp << endl);
UINFO(9, " const= " << fromConstp << endl);
return castable;
}
static Castable computeCastableImp(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
const AstNode* fromConstp) {
const Castable castable = UNSUPPORTED;
Expand Down Expand Up @@ -7536,6 +7505,15 @@ class WidthVisitor final : public VNVisitor {
return userIterateSubtreeReturnEdits(nodep, WidthVP{SELF, BOTH}.p());
}
~WidthVisitor() override = default;

static Castable computeCastable(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
const AstNode* fromConstp) {
const auto castable = computeCastableImp(toDtp, fromDtp, fromConstp);
UINFO(9, " castable=" << castable << " for " << toDtp << endl);
UINFO(9, " =?= " << fromDtp << endl);
UINFO(9, " const= " << fromConstp << endl);
return castable;
}
};

//######################################################################
Expand All @@ -7554,6 +7532,33 @@ void V3Width::width(AstNetlist* nodep) {
V3Global::dumpCheckGlobalTree("width", 0, dumpTreeLevel() >= 3);
}

AstNodeDType* V3Width::getCommonClassTypep(AstNode* nodep1, AstNode* nodep2) {
// Return the class type that both nodep1 and nodep2 are castable to.
// If both are null, return the type of null constant.
// If one is a class and one is null, return AstClassRefDType that points to that class.
// If no common class type exists, return nullptr.

// First handle cases with null values and when one class is a super class of the other.
if (VN_IS(nodep1, Const)) std::swap(nodep1, nodep2);
const Castable castable
= WidthVisitor::computeCastable(nodep1->dtypep(), nodep2->dtypep(), nodep2);
if (castable == SAMEISH || castable == COMPATIBLE) {
return nodep1->dtypep();
} else if (castable == DYNAMIC_CLASS) {
return nodep2->dtypep();
}

AstClassRefDType* classDtypep1 = VN_CAST(nodep1->dtypep(), ClassRefDType);
while (classDtypep1) {
const Castable castable
= WidthVisitor::computeCastable(classDtypep1, nodep2->dtypep(), nodep2);
if (castable == COMPATIBLE) return classDtypep1;
AstClassExtends* const extendsp = classDtypep1->classp()->extendsp();
classDtypep1 = extendsp ? VN_AS(extendsp->dtypep(), ClassRefDType) : nullptr;
}
return nullptr;
}

//! Single node parameter propagation
//! Smaller step... Only do a single node for parameter propagation
AstNode* V3Width::widthParamsEdit(AstNode* nodep) {
Expand Down
3 changes: 3 additions & 0 deletions src/V3Width.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

class AstNetlist;
class AstNode;
class AstNodeDType;

//============================================================================

Expand All @@ -33,6 +34,8 @@ class V3Width final {
// Final step... Mark all widths as equal
static void widthCommit(AstNetlist* nodep);

static AstNodeDType* getCommonClassTypep(AstNode* nodep1, AstNode* nodep2);

// For use only in WidthVisitor
// Replace AstSelBit, etc with AstSel/AstArraySel
// Returns replacement node if nodep was deleted, or null if none.
Expand Down
21 changes: 21 additions & 0 deletions test_regress/t/t_class_if_assign.pl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#!/usr/bin/env perl
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2022 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0

scenarios(simulator => 1);

compile(
);

execute(
check_finished => 1,
);

ok(1);
1;
43 changes: 43 additions & 0 deletions test_regress/t/t_class_if_assign.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed into the Public Domain, for any use,
// without warranty, 2023 by Antmicro Ltd.
// SPDX-License-Identifier: CC0-1.0

class Cls;
int x;
function new;
x = 1;
endfunction
endclass

class ExtendCls extends Cls;
function new;
x = 2;
endfunction
endclass

class AnotherExtendCls extends Cls;
function new;
x = 3;
endfunction
endclass

module t (/*AUTOARG*/);
initial begin
Cls cls = new;
ExtendCls ext_cls = new;
AnotherExtendCls an_ext_cls = new;

if (cls.x == 1) cls = ext_cls;
else cls = an_ext_cls;
if (cls.x != 2) $stop;

if (cls.x == 1) cls = ext_cls;
else cls = an_ext_cls;
if (cls.x != 3) $stop;

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

0 comments on commit 2d9bc73

Please sign in to comment.