From 248bd173d3e7c044ec0c9b0b46a04fac4ac72f86 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 28 Jan 2023 18:06:37 -0500 Subject: [PATCH] Support interface classes and class implements. --- Changes | 1 + src/V3LinkDot.cpp | 71 +++++++++++++++---- src/V3Width.cpp | 2 +- src/V3WidthCommit.h | 5 +- src/verilog.y | 5 +- test_regress/t/t_class_extends_bad.out | 3 +- test_regress/t/t_implements.out | 14 ---- test_regress/t/t_implements.pl | 6 -- test_regress/t/t_implements.v | 34 ++++----- test_regress/t/t_implements_collision.out | 5 -- test_regress/t/t_implements_collision.pl | 6 +- test_regress/t/t_implements_collision.v | 2 +- test_regress/t/t_implements_collision_bad.out | 8 ++- test_regress/t/t_implements_contents_bad.out | 8 +-- test_regress/t/t_implements_missing_bad.out | 10 +-- test_regress/t/t_implements_new_bad.out | 8 +-- test_regress/t/t_implements_noinherit_bad.out | 7 +- .../t/t_implements_noninterface_bad.out | 4 -- 18 files changed, 105 insertions(+), 94 deletions(-) delete mode 100644 test_regress/t/t_implements.out delete mode 100644 test_regress/t/t_implements_collision.out diff --git a/Changes b/Changes index 47232dc75..a4e2ea12d 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,7 @@ Verilator 5.007 devel **Minor:** * Support unpacked unions. +* Support interface classes and class implements. * Support global clocking and $global_clock. diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index f290817d0..7eb296357 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -748,7 +748,6 @@ class LinkDotFindVisitor final : public VNVisitor { string m_scope; // Scope text const AstNodeBlock* m_blockp = nullptr; // Current Begin/end block const AstNodeFTask* m_ftaskp = nullptr; // Current function/task - bool m_inInterfaceClass = false; // Inside a class interface bool m_inRecursion = false; // Inside a recursive module int m_paramNum = 0; // Parameter number, for position based connection bool m_explicitNew = false; // Hit a "new" function @@ -917,9 +916,6 @@ class LinkDotFindVisitor final : public VNVisitor { void visit(AstClass* nodep) override { UASSERT_OBJ(m_curSymp, nodep, "Class not under module/package/$unit"); UINFO(8, " " << nodep << endl); - if (nodep->isInterfaceClass()) { - nodep->v3warn(E_UNSUPPORTED, "Unsupported: interface classes"); - } // Remove classes that have void params, as they were only used for the parameterization // step and will not be instantiated if (m_statep->removeVoidParamedClasses()) { @@ -934,7 +930,6 @@ class LinkDotFindVisitor final : public VNVisitor { } VL_RESTORER(m_scope); VL_RESTORER(m_classOrPackagep); - VL_RESTORER(m_inInterfaceClass); VL_RESTORER(m_modSymp); VL_RESTORER(m_curSymp); VL_RESTORER(m_paramNum); @@ -945,7 +940,6 @@ class LinkDotFindVisitor final : public VNVisitor { VSymEnt* const upperSymp = m_curSymp; m_scope = m_scope + "." + nodep->name(); m_classOrPackagep = nodep; - m_inInterfaceClass = nodep->isInterfaceClass(); m_curSymp = m_modSymp = m_statep->insertBlock(upperSymp, nodep->name(), nodep, m_classOrPackagep); m_statep->insertMap(m_curSymp, m_scope); @@ -1074,9 +1068,11 @@ class LinkDotFindVisitor final : public VNVisitor { VL_RESTORER(m_curSymp); VSymEnt* upSymp = m_curSymp; { - if (m_inInterfaceClass && !nodep->pureVirtual()) { + if (VN_IS(m_curSymp->nodep(), Class) + && VN_AS(m_curSymp->nodep(), Class)->isInterfaceClass() && !nodep->pureVirtual() + && !nodep->isConstructor()) { nodep->v3error("Interface class functions must be pure virtual" - << " (IEEE 1800-2017 8.26)"); + << " (IEEE 1800-2017 8.26): " << nodep->prettyNameQ()); } // Change to appropriate package if extern declaration (vs definition) if (nodep->classOrPackagep()) { @@ -1200,10 +1196,10 @@ class LinkDotFindVisitor final : public VNVisitor { // Var: Remember its name for later resolution UASSERT_OBJ(m_curSymp && m_modSymp, nodep, "Var not under module?"); iterateChildren(nodep); - if (m_inInterfaceClass && !nodep->isParam() && !nodep->isFuncLocal() - && !nodep->isFuncReturn()) { + if (VN_IS(m_curSymp->nodep(), Class) + && VN_AS(m_curSymp->nodep(), Class)->isInterfaceClass() && !nodep->isParam()) { nodep->v3error("Interface class cannot contain non-parameter members" - << " (IEEE 1800-2017 8.26)"); + << " (IEEE 1800-2017 8.26): " << nodep->prettyNameQ()); } if (!m_statep->forScopeCreation()) { // Find under either a task or the module's vars @@ -2026,6 +2022,7 @@ private: AstNodeFTask* m_ftaskp = nullptr; // Current function/task int m_modportNum = 0; // Uniqueify modport numbers bool m_inSens = false; // True if in senitem + std::set m_ifClassImpNames; // Names imported from interface class struct DotStates { DotPosition m_dotPos; // Scope part of dotted resolution @@ -2179,7 +2176,45 @@ private: } while (classSymp && !VN_IS(classSymp->nodep(), Class)); return classSymp; } - + void importImplementsClass(AstClass* implementsClassp, VSymEnt* interfaceSymp, + AstClass* interfaceClassp) { + UINFO(8, "importImplementsClass to " << implementsClassp << " from " << interfaceClassp + << endl); + for (VSymEnt::const_iterator it = interfaceSymp->begin(); it != interfaceSymp->end(); + ++it) { + if (AstNode* interfaceSubp = it->second->nodep()) { + UINFO(8, " SymFunc " << interfaceSubp << endl); + if (VN_IS(interfaceSubp, NodeFTask)) { + bool existsInChild = m_curSymp->findIdFlat(interfaceSubp->name()); + if (!existsInChild && !implementsClassp->isInterfaceClass()) { + implementsClassp->v3error( + "Class " << implementsClassp->prettyNameQ() << " implements " + << interfaceClassp->prettyNameQ() + << " but is missing implementation for " + << interfaceSubp->prettyNameQ() << " (IEEE 1800-2017 8.26)\n" + << implementsClassp->warnContextPrimary() << '\n' + << interfaceSubp->warnOther() + << "... Location of interface class's function\n" + << interfaceSubp->warnContextSecondary()); + } + if (m_ifClassImpNames.find(interfaceSubp->name()) != m_ifClassImpNames.end() + && !existsInChild) { + implementsClassp->v3error( + "Class " << implementsClassp->prettyNameQ() << " implements " + << interfaceClassp->prettyNameQ() + << " but missing inheritance conflict resolution for " + << interfaceSubp->prettyNameQ() + << " (IEEE 1800-2017 8.26.6.2)\n" + << implementsClassp->warnContextPrimary() << '\n' + << interfaceSubp->warnOther() + << "... Location of interface class's function\n" + << interfaceSubp->warnContextSecondary()); + } + m_ifClassImpNames.emplace(interfaceSubp->name()); + } + } + } + } // VISITs void visit(AstNetlist* nodep) override { // Recurse..., backward as must do packages before using packages @@ -3186,15 +3221,21 @@ private: checkNoDot(nodep); VL_RESTORER(m_curSymp); VL_RESTORER(m_modSymp); + VL_RESTORER(m_ifClassImpNames); { m_ds.init(m_curSymp); // Until overridden by a SCOPE m_ds.m_dotSymp = m_curSymp = m_modSymp = m_statep->getNodeSym(nodep); m_modp = nodep; + int next = 0; for (AstNode* itemp = nodep->extendsp(); itemp; itemp = itemp->nextp()) { if (AstClassExtends* const cextp = VN_CAST(itemp, ClassExtends)) { // Replace abstract reference with hard pointer // Will need later resolution when deal with parameters + if (++next == 2 && !nodep->isInterfaceClass() && !cextp->isImplements()) { + cextp->v3error("Multiple inheritance illegal on non-interface classes" + " (IEEE 1800-2017 8.13)"); + } if (cextp->childDTypep() || cextp->dtypep()) continue; // Already converted AstClassOrPackageRef* const cpackagerefp = VN_CAST(cextp->classOrPkgsp(), ClassOrPackageRef); @@ -3256,7 +3297,11 @@ private: classp->isExtended(true); nodep->isExtended(true); VSymEnt* const srcp = m_statep->getNodeSym(classp); - m_curSymp->importFromClass(m_statep->symsp(), srcp); + if (classp->isInterfaceClass()) { + importImplementsClass(nodep, srcp, classp); + } else { + m_curSymp->importFromClass(m_statep->symsp(), srcp); + } VL_DO_DANGLING(cpackagerefp->unlinkFrBack()->deleteTree(), cpackagerefp); } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 52a7610f4..cd4545de4 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -3624,7 +3624,7 @@ private: // Either made explicitly or V3LinkDot made implicitly classp->v3fatalSrc("Can't find class's new"); } - if (classp->isVirtual()) { + if (classp->isVirtual() || classp->isInterfaceClass()) { nodep->v3error( "Illegal to call 'new' using an abstract virtual class (IEEE 1800-2017 8.21)"); } diff --git a/src/V3WidthCommit.h b/src/V3WidthCommit.h index 785db8add..34cc4c689 100644 --- a/src/V3WidthCommit.h +++ b/src/V3WidthCommit.h @@ -214,8 +214,9 @@ private: void visit(AstNodeFTask* nodep) override { iterateChildren(nodep); editDType(nodep); - if (nodep->classMethod() && nodep->pureVirtual() && VN_IS(m_modp, Class) - && !VN_AS(m_modp, Class)->isVirtual()) { + AstClass* classp = VN_CAST(m_modp, Class); + if (nodep->classMethod() && nodep->pureVirtual() && classp && !classp->isInterfaceClass() + && !classp->isVirtual()) { nodep->v3error( "Illegal to have 'pure virtual' in non-virtual class (IEEE 1800-2017 8.21)"); } diff --git a/src/verilog.y b/src/verilog.y index 1520945b6..569671371 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -6504,10 +6504,7 @@ classExtendsE: // IEEE: part of class_declaration classExtendsList: // IEEE: part of class_declaration classExtendsOne { $$ = $1; $$ = $1; } - | classExtendsList ',' classExtendsOne - { $$ = $3; $$ = $3; - BBUNSUP($3, "Multiple inheritance illegal on non-interface classes (IEEE 1800-2017 8.13), " - "and unsupported for interface classes."); } + | classExtendsList ',' classExtendsOne { $$ = addNextNull($1, $3); $$ = $3; } ; classExtendsOne: // IEEE: part of class_declaration diff --git a/test_regress/t/t_class_extends_bad.out b/test_regress/t/t_class_extends_bad.out index eed5066dd..468fdb3a7 100644 --- a/test_regress/t/t_class_extends_bad.out +++ b/test_regress/t/t_class_extends_bad.out @@ -1,5 +1,4 @@ -%Error-UNSUPPORTED: t/t_class_extends_bad.v:13:26: Multiple inheritance illegal on non-interface classes (IEEE 1800-2017 8.13), and unsupported for interface classes. +%Error: t/t_class_extends_bad.v:13:26: Multiple inheritance illegal on non-interface classes (IEEE 1800-2017 8.13) 13 | class Cls extends Base1, Base2; | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest %Error: Exiting due to diff --git a/test_regress/t/t_implements.out b/test_regress/t/t_implements.out deleted file mode 100644 index ee4b56b13..000000000 --- a/test_regress/t/t_implements.out +++ /dev/null @@ -1,14 +0,0 @@ -%Error-UNSUPPORTED: t/t_implements.v:7:11: Unsupported: interface classes - 7 | interface class Icempty; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_implements.v:10:11: Unsupported: interface classes - 10 | interface class Icls1; - | ^~~~~ -%Error-UNSUPPORTED: t/t_implements.v:17:11: Unsupported: interface classes - 17 | interface class Iext1 extends Icls1; - | ^~~~~ -%Error-UNSUPPORTED: t/t_implements.v:21:11: Unsupported: interface classes - 21 | interface class Icls2; - | ^~~~~ -%Error: Exiting due to diff --git a/test_regress/t/t_implements.pl b/test_regress/t/t_implements.pl index 40f69d41d..da2e37bda 100755 --- a/test_regress/t/t_implements.pl +++ b/test_regress/t/t_implements.pl @@ -11,13 +11,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); compile( - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); -execute( - check_finished => 1, - ) if !$Self->{vlt_all}; - ok(1); 1; diff --git a/test_regress/t/t_implements.v b/test_regress/t/t_implements.v index 11e2080a7..caad29c82 100644 --- a/test_regress/t/t_implements.v +++ b/test_regress/t/t_implements.v @@ -19,25 +19,25 @@ interface class Iext1 extends Icls1; endclass interface class Icls2; - pure virtual function int icf2; + pure virtual function int icf2(int in); pure virtual function int icfboth; endclass - virtual class Base implements Iext1, Icls2; - virtual function int icf1; - return 1; - endfunction - virtual function int icf101; - return 101; - endfunction - virtual function int icf2; - return 2; - endfunction - virtual function int icfboth; - return 3; - endfunction - pure virtual function int icfpartial; - endclass +virtual class Base implements Iext1, Icls2; + virtual function int icf1; + return 1; + endfunction + virtual function int icf101; + return 101; + endfunction + virtual function int icf2(int in); + return in + 2; + endfunction + virtual function int icfboth; + return 3; + endfunction + pure virtual function int icfpartial; +endclass class Cls extends Base; virtual function int icfpartial; @@ -56,7 +56,7 @@ module t(/*AUTOARG*/); c = new; if (c.icf1() != 1) $stop; if (c.icf101() != 101) $stop; - if (c.icf2() != 2) $stop; + if (c.icf2(1000) != 1002) $stop; if (c.icfpartial() != 62) $stop; i1 = c; diff --git a/test_regress/t/t_implements_collision.out b/test_regress/t/t_implements_collision.out deleted file mode 100644 index 2f499d637..000000000 --- a/test_regress/t/t_implements_collision.out +++ /dev/null @@ -1,5 +0,0 @@ -%Error-UNSUPPORTED: t/t_implements_collision.v:15:41: Multiple inheritance illegal on non-interface classes (IEEE 1800-2017 8.13), and unsupported for interface classes. - 15 | interface class IclsBoth extends Icls1, Icls2; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error: Exiting due to diff --git a/test_regress/t/t_implements_collision.pl b/test_regress/t/t_implements_collision.pl index 40f69d41d..859050d63 100755 --- a/test_regress/t/t_implements_collision.pl +++ b/test_regress/t/t_implements_collision.pl @@ -2,7 +2,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } # DESCRIPTION: Verilator: Verilog Test driver/expect definition # -# Copyright 2019 by Wilson Snyder. This program is free software; you +# Copyright 2023 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. @@ -11,13 +11,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(simulator => 1); compile( - fails => $Self->{vlt_all}, - expect_filename => $Self->{golden_filename}, ); execute( check_finished => 1, - ) if !$Self->{vlt_all}; + ); ok(1); 1; diff --git a/test_regress/t/t_implements_collision.v b/test_regress/t/t_implements_collision.v index 12ce71a8f..d0032ade0 100644 --- a/test_regress/t/t_implements_collision.v +++ b/test_regress/t/t_implements_collision.v @@ -28,7 +28,7 @@ module t(/*AUTOARG*/); initial begin c = new; - if (c.ifcboth() != 3) $stop; + if (c.icfboth() != 3) $stop; $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_implements_collision_bad.out b/test_regress/t/t_implements_collision_bad.out index 96cfc7cec..09e7a792a 100644 --- a/test_regress/t/t_implements_collision_bad.out +++ b/test_regress/t/t_implements_collision_bad.out @@ -1,5 +1,7 @@ -%Error-UNSUPPORTED: t/t_implements_collision_bad.v:15:41: Multiple inheritance illegal on non-interface classes (IEEE 1800-2017 8.13), and unsupported for interface classes. +%Error: t/t_implements_collision_bad.v:15:11: Class 'IclsBoth' implements 'Icls2' but missing inheritance conflict resolution for 'icfboth' (IEEE 1800-2017 8.26.6.2) 15 | interface class IclsBoth extends Icls1, Icls2; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest + | ^~~~~ + t/t_implements_collision_bad.v:12:30: ... Location of interface class's function + 12 | pure virtual function int icfboth; + | ^~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_implements_contents_bad.out b/test_regress/t/t_implements_contents_bad.out index be41f2343..642eec558 100644 --- a/test_regress/t/t_implements_contents_bad.out +++ b/test_regress/t/t_implements_contents_bad.out @@ -1,11 +1,7 @@ -%Error-UNSUPPORTED: t/t_implements_contents_bad.v:7:11: Unsupported: interface classes - 7 | interface class Icls; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error: t/t_implements_contents_bad.v:8:8: Interface class cannot contain non-parameter members (IEEE 1800-2017 8.26) +%Error: t/t_implements_contents_bad.v:8:8: Interface class cannot contain non-parameter members (IEEE 1800-2017 8.26): 'badi' 8 | int badi; | ^~~~ -%Error: t/t_implements_contents_bad.v:9:9: Interface class functions must be pure virtual (IEEE 1800-2017 8.26) +%Error: t/t_implements_contents_bad.v:9:9: Interface class functions must be pure virtual (IEEE 1800-2017 8.26): 'badtask' 9 | task badtask; | ^~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_implements_missing_bad.out b/test_regress/t/t_implements_missing_bad.out index 6c57b4956..67c410cc6 100644 --- a/test_regress/t/t_implements_missing_bad.out +++ b/test_regress/t/t_implements_missing_bad.out @@ -1,5 +1,7 @@ -%Error-UNSUPPORTED: t/t_implements_missing_bad.v:7:11: Unsupported: interface classes - 7 | interface class Icls1; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_implements_missing_bad.v:12:1: Class 'Cls' implements 'Icls1' but is missing implementation for 'icf2' (IEEE 1800-2017 8.26) + 12 | class Cls implements Icls1; + | ^~~~~ + t/t_implements_missing_bad.v:9:30: ... Location of interface class's function + 9 | pure virtual function int icf2; + | ^~~~ %Error: Exiting due to diff --git a/test_regress/t/t_implements_new_bad.out b/test_regress/t/t_implements_new_bad.out index 2cd806fe6..e86450d3a 100644 --- a/test_regress/t/t_implements_new_bad.out +++ b/test_regress/t/t_implements_new_bad.out @@ -1,5 +1,5 @@ -%Error-UNSUPPORTED: t/t_implements_new_bad.v:7:11: Unsupported: interface classes - 7 | interface class Icls; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_implements_new_bad.v:13:11: Illegal to call 'new' using an abstract virtual class (IEEE 1800-2017 8.21) + : ... In instance t + 13 | c = new; + | ^~~ %Error: Exiting due to diff --git a/test_regress/t/t_implements_noinherit_bad.out b/test_regress/t/t_implements_noinherit_bad.out index 813639b52..d93b92911 100644 --- a/test_regress/t/t_implements_noinherit_bad.out +++ b/test_regress/t/t_implements_noinherit_bad.out @@ -1,5 +1,4 @@ -%Error-UNSUPPORTED: t/t_implements_noinherit_bad.v:7:11: Unsupported: interface classes - 7 | interface class Icls; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: t/t_implements_noinherit_bad.v:14:16: Can't find definition of variable: 'IP' + 14 | $display(IP); + | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_implements_noninterface_bad.out b/test_regress/t/t_implements_noninterface_bad.out index 5bf1d237d..56fc33832 100644 --- a/test_regress/t/t_implements_noninterface_bad.out +++ b/test_regress/t/t_implements_noninterface_bad.out @@ -1,7 +1,3 @@ -%Error-UNSUPPORTED: t/t_implements_noninterface_bad.v:13:11: Unsupported: interface classes - 13 | interface class Icls; - | ^~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest %Error: t/t_implements_noninterface_bad.v:10:26: Attempting to implement from non-interface class 'NotIcls' ... Suggest use 'extends' 10 | class ClsBad1 implements NotIcls;