Original Redmine Comment
Author Name: Alex Duller
Original Date: 2009-07-16T14:10:43Z

I have created a test and edited verilog.l and verilog.y by adding to yINTERFACE and yENDINTERFACE. So, I think I just need instructions on how to go about adding the grammar rules. Thanks!

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-07-16T14:18:40Z

Obviously this is a good chunk of work. I'd urge you to do it in little feature pieces and it won't seem quite so daunting.

Create test cases - you'll want to insure they work on another simulator.
For now I'd avoid modports, parameterized interfaces, or even USING an interface until the simpler stuff works.

Add keywords to verilog.l

Follow how the bison rules related to yINTERFACE work in the verilog-perl parser. Copy(ish) them into verilator. You'll want to see how verilator does module ports. I think it'll be mostly the same, but there might be some differences.

Make the productions ({...} in bison) just be empty {} and get the test case to parse ok. Might want to "git commit" at this point.

Create new AstInterface type in V3Ast.h. AstModule is probably a good start, but most the accessors don't need copying - I think you just need a list of pins will do. (A good reference of what is needed is to look at the language specification VPI for interface, though Verilator doesn't strictly do what it says.) Something like this

struct AstInterface : public AstNode {
     // An interface declaration
     // Parents: module
     // Children: vars (inputs/outputs)
private:
     string      m_name;         // Name of the interface
     string      m_origName;     // Name of the interface, ignoring name() chang$
public:
     AstInterface(FileLine* fl, const string& name)
         : AstNode (fl)
         ,m_name(name), m_origName(name) { }
     ASTNODE_NODE_FUNCS(Interface, INTERFACE)
     virtual void dump(ostream& str);
     virtual bool maybePointedTo() const { return true; }
     virtual string name()       const { return m_name; }
     AstNode*    stmtsp()        const { return op2p()->castNode(); }
     // METHODS
     void addStmtp(AstNode* nodep) { addOp2p(nodep); }
     // ACCESSORS
     void name(const string& name)       { m_name = name; }
     string origName() const     { return m_origName; }
};

Also you need to know when an interface is used. It would be nice to define and use an AstInterfaceRef, but I think the language doesn't allow you to know the difference between a cell and interface until the names are linked with their definitions. So create AstCell and AstPin structures when an interface is used. (To simplify your life, you might want to test that interfaces can be parsed and completely get through verilator before trying to use an interface.) V3Link then would change AstCells to AstInterfaceRef's.

Change the rules to create a new AstInterface + AstCell at the appropriate point, then signals under the interface will get attached to it. See how modules work as an example. When you run verilator with --debug, the .tree files will give you a good idea what's being formed; check the new interfaces seem to have the right info. Another good git commit point.

Now to do something with the interfaces. I haven't thought a lot about it, but think they can get eliminated relatively early by pushing them into the modules that use them. Grep for "AstModule" and you'll get an idea where the code needs to be improved. Correlate this to the list that steps are executed in Verilator.cpp - probably everything after V3Inline is called won't need to know about interfaces.

Some notes

V3Link needs to remember the interface names, and when it sees an AstCell see if the name matches an interface.

V3LinkCells likewise; it's legal to auto-find "interface.v" if you reference "interface" but haven't declared it yet.

V3Dead should remove unused interfaces

V3Param needs to resolve parameterized interfaces

V3Inline is what inlines modules. I'd say use this as an example to make a new V3Interface.cpp that takes in the interfaced tree and flattens the interfaces signals into the modules. Run this before V3Inline by calling it in Verilator.cpp. Again the key is to use --dump to see the .tree that comes in and goes out.

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2009-11-12T11:50:45Z

Attaching a very basic testcase for interfaces that works in another simulator.

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2009-11-12T16:31:39Z

Hi Wilson, I'm working on getting the parser working for the attached testcase, the interface counter_io looks like:

     1: INTERFACE 0x94d5520 <e288#> {6} w0  counter_io  L4
     1:2: VAR 0x94e3aa0 <e10#> {7} w0  value VAR
     1:2:1: BASICDTYPE 0x94e38a0 <e11#> {7} w1  logic[] [logic]
     1:2:1:1: RANGE 0x94e3910 <e9#> {7} w0
     1:2:1:1:2: CONST 0x94e3980 <e7#> {7} swu32/2  ?32?sh3
     1:2:1:1:3: CONST 0x94e3a10 <e8#> {7} swu32/1  ?32?sh0
     1:2: VAR 0x94e3bf8 <e19#> {8} w0  reset VAR
     1:2:1: BASICDTYPE 0x94e3b88 <e18#> {8} w1  logic [logic]

and when the interface is instantiated in the counter module:

     1: MODULE 0x94e7988 <e287#> {45} w0  counter  L3
     1:2: PORT 0x94e7a98 <e227#> {46} w0  clkm
     1:2: VAR 0x94e7b78 <e231#> {46} w0  clkm [I] INPUT
     1:2:1: BASICDTYPE 0x94e7b08 <e230#> {46} w1  logic [logic] [IMPLICIT]
     1:2: CELL 0x94e7c80 <e233#> {47} w0  c_data -> INTERFACE 0x94d5520 <e288#> {6} w0  counter_io  L4

However when counter is instantiated in the module t, it sees the interface as just another pin:

     1:2: CELL 0x94e4848 <e62#> {21} w0  c1 -> MODULE 0x94e7988 <e287#> {45} w0  counter  L3
     1:2:1: PIN 0x94e4628 <e53#> {21} w0  clkm ->UNLINKED
     1:2:1:1: PARSEREF 0x94e45b8 <e54#> {21} w0 [VAR_ANY]
     1:2:1:1:1: TEXT 0x94e4548 <e52#> {21} w0
     1:2:1: PIN 0x94e47d0 <e60#> {22} w0  c_data ->UNLINKED
     1:2:1:1: PARSEREF 0x94e4760 <e59#> {22} w0 [VAR_ANY]
     1:2:1:1:1: TEXT 0x94e46f0 <e57#> {22} w0

The three errors I'm getting are:

(1) %Error: t/t_interface.v:22: Pin not found: c_data
(2) %Error: t/t_interface.v:22: Can't find definition of signal: c1_data
(3) %Error: t/t_interface.v:22: Unsupported in C: Variable has same name as cell: c1_data

I don't think (3) is a problem because the cell name will disappear later on. To solve (2), I think I need to expand interface cells to include PIN and VAR nodes, can this happen in V3Link? I'm not sure about (1), do I need to find some way of replacing the PIN with an interface cell and link them together? Does it sound as if I'm going along the right lines here?

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-11-12T18:32:40Z

Hmm, I'll make a stab, but not having done it before I suspect the approach will need adjusting as you discover more. I guess interfaces are both module like and also data-type (DType) like, in that they can be accessed like structures. (Though cells also work this way of course. In retrospect cells should probably have been a instantiation of a Module datatype.)

Looking at counter's c_value, I'm wondering if it's right to have c_value as a cell. I think it might be closer to be a var (or new AstNode type) with dtype that references the interface (pointing to a InterfaceDType similar to RefDType)

Then in t, it would remain a pin that connects to counter's c_value var, just as normal; what's special is it connects to cl.c_value, who's dtypep() is InterfaceDType.

t.cl would still be a Cell (or InterfaceCell if you want to abstract it), if you need to know the difference V3Link could change from Cell to InterfaceCell.

Later V3Inline/V3Cell would detect the pin's connecting cl_data->InterfaceCell->counter_io to cl->counter->Var->dtype->InterfaceDType->counter_io. Both are counter_io's so it's right.
(Again if everything was a dtype it would make more sense, because then it would just detect the dtype()'s match.)

Also, give some thought to how modports would fit into this scheme.

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-02-01T17:07:57Z

Three patches attached to add support for interfaces and modports. There are a number of open issues with these patches:

• There are almost no lint checks on interfaces/modports, these need to be added.
• The inline optimization is disabled for modules that contain an interface. I need to make a number of changes to the code to fix this but this should make the code much better.
• Each use of an interface creates a Scope/VarScope under the module, but V3Delayed operates on the Var nodes. There were no free user pointers in V3Delayed to make it operate on the VarScope instead so I added a fifth user pointer. If this isn't acceptable I think we either need to find another way to rework V3Delayed or clone the entire interface module on each instantiation. If the last two patches aren't accepted the only issue will be that some Vars in interfaces may wrongly be identified as having delayed and non-delayed assignments.

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-02-01T23:58:36Z

I applied patch 2, user5, as I didn't see a easy work around, and there's enough visitors squeezed into 4 that it's probably useful.

I also applied 3, it seems safe in isolation from #1.

The change I'd like to see to 1 is to not call ParseResolve twice; this will cause problems. Instead have V3LinkCells understand how to find interfaces (take the code you added from ParseResolve). If I'm missing some key flaw let me know.

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-02-03T16:00:18Z

Wilson Snyder wrote:

The change I'd like to see to 1 is to not call ParseResolve twice; this will cause problems. Instead have V3LinkCells understand how to find interfaces (take the code you added from ParseResolve). If I'm missing some key flaw let me know.

I'm assuming by ParseResolve you mean the second call to V3LinkCells::link(). The second call to V3LinkCells is there because this Cell is only created in V3LinkParse. This is only needed in the case where there is an interface in the port list of a top-level module.

I've attached a testcase for this, if module t wasn't the top-level, the interface would be created as a cell elsewhere and we'd just link to it. But because t is a top-level module, the cell doesn't exist yet. I could probably create the Interface Cell inside V3LinkLevel instead of V3LinkParse but that's still too late for V3LinkParse.

I might be able to create a new Cell for every interface in a port, mark them as temporary, then remove all of them outside the top-level later but that doesn't feel right either.

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-02-03T21:12:05Z

Ignore my last update. The new Cell for the top-level interface is created from an InterfaceDType after it has been linked, so I can just take the NodeModule pointer from there. I've attached a patch to go on top of 0001-Support-interfaces-and-modports.patch. The testcase included only does basic linting but needs to be extended to check trace and probably connectivity with a custom .cpp file.

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-02-04T03:26:46Z

Attached is a new patch set with my edits; please build off that, it replaces the previous patches.

1. Random :) spacing and minor changes.

2. verilog.y

I try to avoid CRELINE() and instead use a fileline from a token where
possible, as it gets closer to the right line number when there's lots of
line breaks in strange places.

3. V3LinkLevel:

• AstVarRef* rhsp = new AstVarRef(newvarp->fileline(), newvarp->name(), false);
• rhsp->varp(newvarp);
• rhsp->widthSignedFrom(newvarp);

I replaced this with

• AstVarRef* rhsp = new AstVarRef(newvarp->fileline(), newvarp, false);

4. V3AstNodes.h

• AstVarType varType(AstVarType::en type) { m_varType == type; }

• void varType(AstVarType type) { m_varType = type; }

What you had was a NOP. Although I'm a bit worried about doing this at
all, because the type sets a bunch of things up front; I'm not sure changing
it later will work.

5. V3Inline

Added statistic so we know it didn't inline due to unsupported interfaces.

6. V3LinkCells

Put common code into a function.

Now the questions

1. I'm a little surprised you needed a AstModportVar versus using a normal
   Var, but can live with it, esp as AstVar is getting out of control. Was
   there a problem, or did it just seem cleaner?

2. Can you briefly describe the general flow between V3Inst, V3LinkDot, and
   V3Scope and esp how AstAssignVarScope and AstScopeAlias fits into this?
   I think I have an idea, but am missing your take and esp why that was the approach
   selected.

I'm leaning back towards your original suggestion of releasing without
interfaces so you have time to stabilize. Thoughts?

Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-02-04T11:16:12Z

Wilson Snyder wrote:

Now the questions

1. I'm a little surprised you needed a AstModportVar versus using a normal
   Var, but can live with it, esp as AstVar is getting out of control. Was
   there a problem, or did it just seem cleaner?

It seems to me like modports just assign an input/output to a Var and the same
Var gets used regardless of the modport. So a ModportVar just links to a Var and
contains an input/output attribute, it will only be used for linting.

2. Can you briefly describe the general flow between V3Inst, V3LinkDot, and
   V3Scope and esp how AstAssignVarScope and AstScopeAlias fits into this?
   I think I have an idea, but am missing your take and esp why that was the approach
   selected.

• V3Inst - Where an interface is connected between two modules, create an
  AssignVarScope that links the Var->InterfaceDType nodes on both sides (A VarRef
  and a VarXRef)
• V3LinkDot - The idea here is to insert cells/subcells into the graph so the
  normal linking can be used and links straight to the Vars/VarScopes in the
  Interface. This is made more complicated by the fact that entire interfaces can
  be passed down multiple levels of hierarchy.
• V3LinkDot - Save the scope names->pointers in one map and save the
  InterfaceDType->cellVxp in another. Use this at the end of LinkDotFindVisitor to
  insert interfaces as subcells.
• V3Scope - Find the Vars that link to interfaces and add a ScopeAlias node
  instead of a VarScope. These ScopeAlias nodes will be used by V3LinkDot to
  insert subcells to the interfaces.
• V3LinkDot(scoped) - AssignVarScope nodes are used to create a map of
  scopenames to the cellVxp and scopenames to scopenames (for interfaces passed
  down multiple levels) and the ScopeAlias nodes use this map to find the subcells
  to insert. After this the graph contains interfaces as subcells and normal
  linking can be used.

With what I know now, I think this was the wrong way to do it. It's pretty ugly
and makes inlining almost impossible, I should have made this work with inlining
enabled first then disabled not the other way around.

I'm leaning back towards your original suggestion of releasing without
interfaces so you have time to stabilize. Thoughts?

Agreed, I think the Inst/Scope/LinkDot stages will change a lot and it's not as
if 3.800 will be lacking in new features without this :)

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-05-28T01:41:28Z

Interface support added to git towards 3.848.

Interfaces and modports, including with generated data types are supported. Generate blocks around modports are not supported, nor are virtual interfaces nor unnamed interfaces; these seem consistent with synthesis limitations.

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2013-06-02T18:54:12Z

In 3.850.