GSoC 2023 Project

[mole99]

Recently, GSoC 2023 was announced. It would be great if a project for Icarus Verilog could be submitted.

I already talked in #746 about how this could work, but I am opening this issue for greater visibility.

FOSSi Foundation

The FOSSi Foundation is an umbrella organization for GSoC. Their list of previous project ideas shows that a project based on Icarus would fit in pretty well.

Submitting a project via the FOSSi Foundation would reduce the admistratice burden on the mentor as written on their website.

In this way we enable experienced community members to mentor mentees while the FOSSi Foundation takes care of the administrative parts of GSoC.

Mentor

There needs to be a mentor available for the project who has at least 2 hours a week available for mentoring.

For more information see the FOSSi Foundation GSoC House Rules.

Project Idea

I personally would like to work on the SDF support of Icarus. The ultimate goal would be to get Icarus' SDF support in a working state for the OpenMPW program. This is a worthwhile goal as the program becomes increasingly popular and now covers even more PDKs.

Please tell me what you think about all this :)

[mole99]

Hi @steveicarus, I'm pinging you to make sure you see this :)

Around January, the FOSSi Foundation asks for project ideas. There's still a lot of time before then, but time goes fast, especially in December. It probably makes sense to prepare a project idea a bit earlier if the intend to participate in GSoC is there.

If I can help in any way, please let me know.

[red0bear]

Where is community to put money on icarus on these days.

[mole99]

What do you mean, @red0bear?

While I agree with you that people should contribute when they can, be it with money, by opening bug reports or by writing code, I don't see exactly the connection to GSoC?

GSoC is sponsored by Google to allow students, and more recently even beginners in open source software development, to contribute to projects that interest them under the guidance of mentors. It allows students to dedicate their time during the summer to open source projects without having to worry about finances.

Of course, Google isn't doing this because they're just being nice, but because they too rely on open source software to a large extent ;)

In summary, participating in GSoC does not cost Icarus any money. It allows students to dedicate their time working on Icarus while being guided by a mentor.

I hope this made it a bit clearer.

[red0bear]

Google never was nice with anyone in globe. If they are good people icarus could be added there long time ago. Work with this is very frustrating. But what make things good is when community put money on it instead ask for a big corporation to do it. But thats only my personal opinion.

[steveicarus]

I have mentored GSoC projects for Icarus Verilog in the past, but I don't have the bandwidth to do so this go-round. If you can round up someone willing to do the mentoring, I'll gladly look at and approve any project you want to don.

[steveicarus]

@caryr has worked on the SDF part of things, so I suggest that you ping him to see if he's willing to mentor you. (He as also done GSoC mentoring in the past.)

[caryr]

I may be able to mentor, but have been very busy with work and the rest of my life the last couple years. I remember you talking about the SDF improvements in the other PR.

Lets start with figuring out what you think is critical to implement, because some of this will require significant improvements in the VPI interface and VVP internals to allow the SDF annotation to work properly. Also I would prefer someone else drive getting us sponsored. I can act as the figure head and mentor of record, but expect to continue to be very busy for the first half of next year and likely longer.

I should have some time over the next month to work on a proposal, but I want to make sure this is successful for the student so this may require those of with more experience to help out with the VVP internals. @steveicarus this is likely where you would be most helpful/needed (e.g. wire delays and how those get placed relative to the drivers). I think as a team we could make this work, but I don't think it is something I can support entirely on my own.

We also have to be ready to support related bugs as we start more completely testing gate level simulations. I think we are in a relatively good place and SDF annotation is the primary limitation, but we do need to make sure other items do not get in the way of the SDF work.

[caryr]

Another candidate for GSOC work is there are a number of items missing to allow proper mixed signal modelling, but this is likely a multi year project where different pieces are implements. I have bits and pieces of this started, but the big missing item is unpacked structures along with nettypes and efficient resolution functions. Think of a Thevenin type with a drive voltage and resistance that is an unpacked structure with two reals. You then need to be able to declare this as a nettype and give it a resolution function to resolve multiple drivers. The resolution function is written in Verilog code, but needs to be very efficient when executed since it is run each time one of the driver nets changes a value.

Adding DPI support while not required would also be nice.

[vrajeshprakhya]

Is this form for Real Number modeling now supported on Icarus Verilog? (UDNs and resolution functions with multiple drivers on a wreal/UDN?)

[caryr]

Not yet. Single driver RNM is supported, but we have made no progress on UDNs.

[vrajeshprakhya]

Got it - are there any open source/open to use simulators out there that support UDNs and multiple drivers that you may know of?

[caryr]

I am not aware of any free simulators that support UDNs and the commercial simulators I use require a more expensive license to use them so I doubt they would be available in the free demo versions that may be provided.

Mixed signal and RNM in general is a big interest of mine, but most of the time I have for Icarus is spent fixing bugs, not working on enhancements. The details I provided above explain why this is a significant development effort.

[mole99]

Thank you @steveicarus and @caryr for your replies.

I understand that there is only so much time available, and mentoring takes up another chunk of that time. But I also don't think that I will need full-time mentoring, just someone to point me in the right direction.

It would be great if you caryr could be the official mentor of the project. I completely agree that it would be good to have multiple contact persons so that the individual load decreases and I can ask my questions more targeted.

Maybe there are a few more people who are familiar with the internals of Icarus and wouldn't mind answering some of my questions from time to time.

On to what I think is important to implement:

I had a look at some SDF file generated by the OpenLane flow. The missing features are in line with your comment on #746. There are three categories not yet supported in Icarus:

• wire/interconnect delays
• conditional path delays
• timing checks

As an example, I have two SDF files generated from one of my designs. One for the SKY130A PDK and the other one for the GF180 PDK:

wfg_top_sky130.sdf (wfg_top_sky130.zip)

wfg_top_gf180.sdf (wfg_top_gf180.zip)

From what I see in wfg_top_sky130, Icarus needs to support INTERCONNECT to simulate designs for SKY130.

wfg_top_gf180 additionally uses conditional path delays with the keyword COND. This would then be the next priority in order to support GF180.

Both SDF files use timing checks that would be ignored initially, but if there is enough time, an implementation for the following timing checks could be considered:

Timing Checks:

• setup
• hold
• recovery
• removal

All in all, the rough order of work would be as follows:

1. INTERCONNECT
2. COND
3. TIMINGCHECK

If possible, I would really like to work on SDF support :)

Of course there are also interesting topics like AMS or DPI support, but for me personally SDF support would be the most interesting to work on because of the ongoing OpenMPW shuttles and the recent releases of open source PDKs.

[caryr]

I'm sure we can come up with enough time to support you to be successful. Like I said previously I'm okay being the mentor of record. I will certainly need some help from others, but I don't expect that to be too much of a problem.

Have you verified that Icarus parses all the SDF files and generates appropriate warning messages or are there constructs that cause errors? If there are other errors do they fall into the existing categories or are there other missing constructs?

[mole99]

That sounds great :) I'm looking forward to working on this project.

I haven't checked it by simulation yet, which I should have done in hindsight. I have now run a simulation for wfg_top_sky130.sdf:

Icarus parses the file and generates warning messages.
The output consists of two types of warnings. Once for INTERCONNECT:

wfg_top_sky130.sdf:18: SDF WARNING: INTERCONNECT not supported.
...
wfg_top_sky130.sdf:37063: SDF WARNING: INTERCONNECT not supported.

And other warnings in the form of:

SDF WARNING: top.sv:53: Unable to match ModPath A -> Y in tb.wfg_top._10050_
...
SDF WARNING: top.sv:53: Unable to match ModPath A -> X in tb.wfg_top.clkbuf_opt_4_0_io_wbs_clk

Which I am not sure if they are issued due to the unimplemented INTERCONNECT or because of a problem with IOPATH.

The entries for _10050_ in wfg_top_sky130.sdfare:

    (INTERCONNECT _10050_.Y _10066_.A1 (0.000:0.000:0.000) (0.000:0.000:0.000))
    (INTERCONNECT _10050_.Y _10069_.A (0.000:0.000:0.000) (0.000:0.000:0.000))
    (INTERCONNECT _10050_.Y _10091_.B1 (0.000:0.000:0.000) (0.000:0.000:0.000))
...
    (INTERCONNECT _20898_.Q _10050_.A (0.000:0.000:0.000) (0.000:0.000:0.000))
...
 (CELL
  (CELLTYPE "sky130_fd_sc_hd__inv_2")
  (INSTANCE _10050_)
  (DELAY
   (ABSOLUTE
    (IOPATH A Y (0.100:0.100:0.100) (0.075:0.075:0.075))
   )
  )
 )

The instantiation of _10050_ in wfg_top_sky130.v is:

 sky130_fd_sc_hd__inv_2 _10050_ (.A(\wfg_drive_spi_top.wfg_drive_spi.cur_state[1] ),
    .VGND(vssd1),
    .VNB(vssd1),
    .VPB(vccd1),
    .VPWR(vccd1),
    .Y(_02696_));

There are also no warnings issued that timing checks are not yet supported. This would be another point on the list, to first add a warning for unsupported timing checks.

I have also tried to simulate for GF180 but I did not yet manage to do so. There are some missing modules which I can not find anywhere in the PDK. There are also no example simulations available from Efabless' side yet. I think it makes sense to focus on SKY130 at the beginning.

[caryr]

If all you see is warnings then it is something I have looked at in the past and I usually remember the issues I was facing in the implementation once I start to look at the code again. Timing checks are currently removed by the compiler so I think the SDF annotator may incorrectly silently drops them. Adding an appropriate warning should be trivial.

I think the modpath issue is rooted in the conditional path delay limitations even though this specific instance only has a single path. My guess is the simple cases like this you can implement first and then enhance the code to handle the more complex cases.

Trying to get two larger examples to test with is good. Also look at the existing SDF tests to see how those were implemented so you can understand the methodology.

[mole99]

I see, you previously took the time to turn the crashes due to unsupported features into warnings.

Adding a warning for timing checks may then be a good starting point to familiarize myself with the codebase.

Yes I agree the SDF file from the GF180 process node would also be very useful for development. I will make sure that I get it to simulate, at the latest by the start of the project.

I will take a look at the SDF tests in Icarus.

Regarding the organizational:

I just saw that the FOSSi Foundation requires at least two mentors listed. While many projects from 2022 only have one mentor listed, I think it would be better to list two as per the FOSSi Foundation GSoC House Rules. Would it be possible to add you @steveicarus alongside caryr to the mentors list? Just for the official part of course.

The application seems to consist only of a PR in their Git repository with a description of the project, skill level, duration, language/tools and the mentors. I would like to do the project with a time commitment of ~350 hours.

Please see the PR for last year's GSoC ideas: fossi-foundation/fossi-foundation.github.io#52

Caryr, you mentioned that you have time to write the proposal in the next month. So you would be handling the project submission to the FOSSi Foundation?

[caryr]

I believe they like to have two listed mentors so that if one needs to drop out for some unforeseen issue the student will not be negatively impacted.

Yes, reviewing the existing code that implements the current SDF annotation would be very beneficial. I try to leave comments regarding what I am working on and the basic issues I was facing. Reviewing these, understanding how the existing code works and understanding the basic work needed to finish the implementation will significantly improve the odds of you having a successful project.

I think most of the parsing is complete, so you will primarily need to work in C/C++. If you feel comfortable using these languages then it sounds like we have a good fit. I will start getting the proposal together next week. I will likely post it here for commenting before officially submitting it to the FOSSi Foundation.

[mole99]

Yes, I expected to work in C/C++.

Awesome, thank you!

[caryr]

I believe that may be the case. We only add back annotation hooks related to the SDF file. Does the standard specify that ``celldefine` has this functionality?

[mole99]

This is what the standard has to say about `celldefine and `endcelldefine:

The directives `celldefine and `endcelldefine tag modules as cell modules. Cells are used by certain
PLI routines and may be useful for applications such as delay calculations. It is advisable to pair each
`celldefine with an `endcelldefine, but it is not required. The latest occurrence of either directive in
the source controls whether modules are tagged as cell modules. More than one of these pairs may appear in
a single source description.
These directives may appear anywhere in the source description, but it is recommended that the directives be
specified outside any design elements.
The `resetall directive includes the effects of a `endcelldefine directive.

I would interpret it that `celldefine has this functionality.

Also it seems like the SKY130 PDK uses it for exactly this purpose, because otherwise how should the simulator know that a module can be annotated with timing information?

Additionally I have looked the the CVC simulator: It has an option +libnocell which deactivates this directive. The help message for it is the following:

+libnocell Ignore `celldefine directives in source and libraries. This
should not be used if delay back annotation is used since it will
probably cause annotation to fail.

This message makes sense considering the previous findings, because delay back annotation will fail if the module has not specified the path via a specify block.

What do you think about my findings?

[mole99]

Well, I was wrong about CVC. Apparently CVC also requires the presence of specify blocks to annotate a cell module.

Here, the sky130_fd_sc_hd standard cell library has been specifically modified to include specify blocks in all standard cells so that they can be annotated: sky130_fd_sc_hd for CVC.

Unfortunately, this doesn't really clarify things. Perhaps for performance reasons CVC has the same restrictions as Icarus Verilog regarding `celldefine and specify? Or `celldefine has another meaning that I do not know yet.

[mole99]

I was wrong after all. The source for the SKY130 PDK has specify blocks for all cells, for example this one: specify for xor3

But these specify blocks are not included by default because Icarus Verilog had issues with parsing some of them. I will look if I can replicate the issue and see whether the parser can be modified to accept the syntax.

[caryr]

Also, by default we ignore all specify blocks, so make sure you are actually including them. If they are not included we should also skip the SDF annotation. Of course there should be a warning for this. I hope once you work is complete we can enable SDF/specify by default.

[mole99]

Yes thanks, I was aware of the -gspecify option.
Does it have performance implications to always have it enabled?

[caryr]

I think it has some because the delay scheduling gets more complicated, but things have changed significantly internally so it may not have the same implications it once did. You may want to benchmark and let us know.

[mole99]

I ran 10 simulations of a medium-complex design, each with and without -gspecify enabled.

On average, the runtimes are similar and I could not really notice a difference. This speaks in favor of enabling SDF/specify by default.

Note: The simulation did not use any specify blocks, so of course this only shows if -gspecify slows down normal simulations. The results make sense because without any specify blocks, no modpaths are added to the design and thus there is no overhead.

[caryr]

Okay, that is good to know. Once we get more of this implemented then we can flip the state of the flag.

[caryr]

I have been reflecting on your project and I want to make sure you don't loose focus which is easy to do when you are early in a project and you have a significant amount of time before the project is over. Reading and reorganizing the documentation is/was a good thing, but that is not part of the proposal or a significant item you will be evaluated on. The same applies to code reorganization. These are distractions from your primary SDF work.

My suggestion is to start focusing on the code. I would start with the either the SDF annotation or parsing the specify blocks in the RTL and make sure it correctly recognizes and for now ignores the missing constructs with an appropriate warning. As you are implementing this start thinking about what is required to implement the missing functionality to meet the requirements in the standard. Then implement one of the smaller items or the simple case for a more complex implementation as you refine your thinking of how to solve the problem in general.

The goal of your project is to correctly parse and back annotate designs using the open source tools and libraries. I will still consider your work a success even if you do not have this 100% implemented because I have a fairly good idea regarding what is needed to implement all the functionality and you may not have enough time to fully implement all the missing functionality, but you will also need to implement significantly more than wire delays and support INTERCONNECT from the SDF file. The fundamental goal from my perspective is to be able to 100% parse the existing libraries and generated SDF files and issue appropriate warnings about any items being ignored. We then want to implement as much of the missing functionality as possible.

If you do not understand something or have no idea where to start please ask. I may not be able to explain exactly what you need to do, but I likely have a general idea regarding what and where things need to be implemented. To me, part of GSOC work is figuring these kind of things out. It's more than just coding what someone else has architected.

I have started reviewing the existing specify/SDF bugs and have some ideas regarding changes in the code that are needed to resolve them. I will try to add my thoughts to the various tickets when I have time.

[mole99]

Yes, I totally agree with you. But it's also important to analyze early on what the obstacles might be for the project, etc., and I now have a pretty good understanding of what needs to be done to simulate the SKY130 PDK.
Now that my university stuff is done, I can focus 100% on the work ahead.

As for the project goals, the official project goals are:

1. Implementing the SDF INTERCONNECT feature
2. Writing tests for the regression suite
3. Example of a SDF back-annotated timing simulation using the SKY130 PDK

But I am ready to go further once these goals are achieved, because my whole incentive is to improve SDF support in Icarus Verilog, and I've set aside the summer to do that.

Over the last few days, I've been working on including the specify blocks into the PDK and getting it working with Icarus Verilog. It turns out that parsing the specify blocks is not a problem at all, but that there is another issue with $recrem and $sethuphold: to support negative timing checks, these directives should generate delayed signals of the reference_event and data_event.

$recrem ( posedge RESET_B , posedge CLK , 0:0:0 , 0:0:0 , notifier , AWAKE , AWAKE , RESETB_delayed , CLK_delayed ) ;
$setuphold ( posedge CLK , posedge D , 0:0:0 , 0:0:0 , notifier , COND0 , COND0 , CLK_delayed , D_delayed ) ;
$setuphold ( posedge CLK , negedge D , 0:0:0 , 0:0:0 , notifier , COND0 , COND0 , CLK_delayed , D_delayed ) ;

Icarus Verilog does not support this yet, which is reflected in warnings like this one:

sky130_fd_sc_hd.v:106606: warning: Timing checks are not supported and delayed signal "CLK_delayed" will not be driven.

But the problem is that these delayed signals are needed in order for the model to work at all. For now, I've worked around this problem by assigning these signals manually:

    assign RESET_B_delayed = RESET_B;
    assign CLK_delayed = CLK;
    assign D_delayed = D;

With these changes, I was able to correctly annotate the rise/fall times for the output of this model. But of course, this is just a workaround.

Timing checks are a big subject in themselves, and I don't think it would make sense to work on implementing $recrem and $setuphold now as well.

But for now there is another solution. Section "15.5.4 Option behavior" in the standard says the following:

As already mentioned, the ability of Verilog simulators to handle negative values in $setuphold and
$recrem timing checks shall be enabled with an invocation option. It is possible models written to accept
negative timing check values with delayed reference and/or delayed data signals can be run without this
invocation option enabled. In this circumstance, the delayed reference and data signals become copies of the
original reference and data signals. The same occurs if an invocation option turning off all timing checks is
used.

That is, the right thing for the simulator to do when timing checks are disabled (or not supported as in our case) is for the delayed signals to simply become copies of the originals.

This means that what I did manually with the assignments, the simulator should do automatically.

So I would suggest implementing this feature before SDF_INTERCONNECT, because:

• It seems to be relatively simple to implement
• It actually improves compatibility with the PDK per the standard
• It involves manipulating the netlist, which I will need for SDF_INTERCONNECT

Do you agree with this?

And yes, it would be great if you could add your thoughts to the open bug reports. To be honest, I still find it difficult to find my way around Icarus Verilog and where and in which way things could be implemented. Once I have a proposal for the SDF_INTERCONNECT feature, I would be happy to discuss it with you, maybe even in a voice call if that is possible for you.

Thanks for your support.

[caryr]

I agree, the simplest way to handle this is to do exactly what you have done in your workaround. If the delayed signal is passed then make it a straight assignment from the appropriate source. You should be able to look at the assign code in the parser and then just duplicate it when this special case exists.

I have been thinking I would see if I can carve out some time to make sure all the specify constructs are handled or implemented in the compiler and even possibly in the runtime, because I do agree you trying to implement all this is too much for a summer project. I was actually thinking about how to fix the modpath race and have been reviewing the requirements for timing checks. Maybe between the two of us we can get this all working for the next release.

[mole99]

Great, then I will work on it next. I will write if any questions concerning the implementation come up.

It would be awesome to eventually get this all working :)
But I would even say, implementing the things needed for SKY130 is already release-worthy.

[mole99]

Short update: I managed to create a proof of concept where the delayed reference and data signals become copies of the original reference and data signals. This works for both $recrem and $setuphold.

I will clean up the code and open a PR on GitHub for further discussion soon. There are certainly some things you will tell me to change and I have some questions too ;)

[mole99]

I have researched what will be required on the VPI side to implement SDF INTERCONNECT. Here are my findings so far.

VPI

Icarus Verilog needs to support the vpiInterModPath object. It is described as an intermodule wire delay, exactly what we need.

Also vpi_handle_multi() must be implemented to get a handle to a vpiInterModPath, like this:

    vpi_handle_multi(vpiInterModPath, port1, port2);

The question is how we can get the ports by their names in the SDF file?

1. Get ports by name

vpi_handle_by_name(name, scope) -> The standard says that since a port does not have a full name it cannot be found by name.

2. Get ports by iterating over all vpiPort objects of a module

So we just get all ports via itr = vpi_iterate(vpiPort, module). But over which module do we iterate? The INTERCONNECT statement contains the hierarchical path to the ports as seen by the current module.

    (INTERCONNECT A submodule.B (0:0:0) (0:0:0))

So port A of the current module is connected to port B of submodule. Therefore, we first get the handle to the correct module and then iterate over the ports.

While iterating over the ports we check if a port has been found with the correct name and width.

Inside vvp the class for ports is called vpiPortInfo and gives us information over:

      __vpiScope *parent_;
      unsigned  index_;
      int       direction_;
      unsigned  width_;
      const char *name_;

Next we need to check if the ports belong to the same net. How do we do that? I am not quite sure yet.

But once we have verified that both ports are connected over the same net, we will create a vpiInterModPath object via vpi_handle_multi.

Finally, we would set the interconnect delays via vpi_put_delays() and to support incremental delays we would also need to support vpi_get_delays().

---

At the moment I'm not quite sure how to implement everything. But I would suggest to start with the VPI code to access the ports and find a way to find the common net.

After this is done, I can focus on implementing vpiInterModPath in vvp.

Some thoughts I had on this were:

An InterModPath is different from a ModPath in that it does not exist until it is first requested. ModPaths are explicitly added via specify blocks, but we don't have an equivalent mechanism for InterModPath. This means that we have to create InterModPaths on the fly.

The idea is, once an InterModPath has been requested for two ports, to get the the net between the two ports and insert a vvp_fun_delay. The InterModPath object then has references to the ports, the net and the delay element. If an output port drives multiple input ports, the net needs to be split into multiple nets.

mod1             mod2
===|            |===
   |      |-----| B
   | net1 |     |===
 A |------|
   |      |     |===
   |      |-----| C
===|            |===
                 mod3

    (INTERCONNECT mod1.A mod2.B (0:0:0) (0:0:0))

mod1                     mod2
===|              net2  |===
   |      |-delay-------| B
   | net1 |             |===
 A |------|
   |      |     |===
   |      |-----| C
===|            |===
                 mod3

@caryr what do you think of the VPI access and my idea to implement the interconnect delay? Do you have any idea to get the common net between two ports? Thanks!

[mole99]

Hi @caryr,

I am able to fully annotate a simple design with buffers connected in series, but I get into problems when one signal drives multiple buffers.

Currently the way I insert an interconnect delay is the following:

1. First, vpi_handle_multi is called with two ports
2. Get the name for each port and find the net with the same name within the current module
3. Get the vvp_net_t object for both vpiNet
4. Compare if it is the same vvp_net_t object
5. Insert an intermodpath between the vvp_net_t and whatever it is connected to

This works well when the buffers are connected in series:

     (reg a;)         (buffer instance) (buffer instance) (buffer instance)
vvp_fun_signal_vec ---> vvp_fun_bufz ---> vvp_fun_bufz ---> vvp_fun_bufz --->

At first a vvp_fun_intermodpath is inserted between vvp_fun_signal_vec and vvp_fun_bufz. The next one is inserted between vvp_fun_bufz and the next vvp_fun_bufz. And finally the last one is inserted between the last vvp_fun_bufz pair. Afterwards the delays can be put into the vvp_fun_intermodpaths.

Problem with buffers connected in parallel

This approach does not work anymore when multiple buffers are driven by the same signal:

Here is a simple design with three buffers in parallel, where different interconnect delays from input a to all three inputs (in) of the buffers should get inserted.

module my_design (
    input a,
    output b
);
    wire w1, w2, w3;

    buffer buffer0 (
        .in (a),
        .out (w1)
    );

    buffer buffer1 (
        .in (a),
        .out (w2)
    );

    buffer buffer2 (
        .in (a),
        .out (w3)
    );
    
    assign b = w1 & w2 & w3;

endmodule

Here is a snippet from the netlist created by VVP.

     (reg a;)         (buffer instances)
vvp_fun_signal_vec ---> vvp_fun_bufz
                    |-> vvp_fun_bufz
                    |-> vvp_fun_bufz

If I now try the same approach as before, I always get the same vvp_net_t (with vvp_fun_signal_vec functor), but I don't know to which of the three vvp_fun_bufz I should connect the vvp_fun_intermodpath to. So three vvp_fun_intermodpath are inserted in series, which is not correct.

When enabling -gspecify this problem does not become easier, now we also have to pair the respective vvp_fun_modpath_src to the vvp_fun_bufz.

     (reg a;)          (modpaths and buffers)
vvp_fun_signal_vec ---> vvp_fun_modpath_src
                    |-> vvp_fun_modpath_src
                    |-> vvp_fun_modpath_src
                    |-> vvp_fun_bufz
                    |-> vvp_fun_bufz
                    |-> vvp_fun_bufz

Proposal

My idea to solve this particular problem is to add a vvp_net_t for each input and output of a module with a vvp_fun_port functor.

This will improve the following points:

• The lookup from vpiPort to vvp_net_t becomes much easier, as the information to which vvp_net_t the port is connected can be stored in vpiPortInfo.
• I can easily check whether the two port functors are connected to each other just by looking at the output ptr of the first one.
• If there is a vvp_fun_intermodpath after the first port and the second port is connected to this same vvp_fun_intermodpath, I know that I have already inserted a vvp_fun_intermodpath and can simply put the delay.
• As a bonus, we can easily implement PORT delays by putting a delay in vvp_fun_port.

The netlist would then look like this:

     (reg a;)          (input port        (input port
                          my_design)          buffers)
vvp_fun_signal_vec ---> vvp_fun_port ---> vvp_fun_port ---> vvp_fun_modpath_src
                                      |                  |-> vvp_fun_bufz
                                      |
                                      |-> vvp_fun_port ---> vvp_fun_modpath_src
                                      |                 |-> vvp_fun_bufz
                                      |
                                      |-> vvp_fun_port ---> vvp_fun_modpath_src
                                                        |-> vvp_fun_bufz

As you can see, the connection to each module is through a vvp_fun_port. This would solve the problem from before where I didn't know which vvp_fun_modpath_src and vvp_fun_bufz belong to the given port.

---

The VVP code generated for one of the buffers currently looks like this:

S_0x55fe0ca10970 .scope module, "buffer0" "buffer" 2 21, 2 3 0, S_0x55fe0ca10790;
 .timescale -9 -12;
    .port_info 0 /INPUT 1 "in";
    .port_info 1 /OUTPUT 1 "out";
L_0x55fe0ca25c30 .functor BUFZ 1, v0x55fe0ca25ac0_0, C4<0>, C4<0>, C4<0>;
v0x55fe0ca10b50_0 .net "in", 0 0, v0x55fe0ca25ac0_0;  alias, 1 drivers
v0x55fe0ca24d70_0 .net "out", 0 0, L_0x55fe0ca25c30;  alias, 1 drivers

I would propose to change it to this:

S_0x556f997bd970 .scope module, "buffer0" "buffer" 2 21, 2 3 0, S_0x556f997bd790;
 .timescale -9 -12;
    .port_info 0 /INPUT 1 "in" L_0x123412341234;
    .port_info 1 /OUTPUT 1 "out" L_0x234234234234;
L_0x123412341234 .functor PORT 1, v0x556f997d2ac0_0, C4<0>, C4<0>, C4<0>;
L_0x234234234234 .functor PORT 1, L_0x556f997d2c30, C4<0>, C4<0>, C4<0>;
L_0x556f997d2c30 .functor BUFZ 1, L_0x123412341234, C4<0>, C4<0>, C4<0>;
v0x556f997bdb50_0 .net "in", 0 0, v0x556f997d2ac0_0;  alias, 1 drivers
v0x556f997d1d70_0 .net "out", 0 0, L_0x556f997d2c30;  alias, 1 drivers

All input/output signals go through the PORT functor, also the .port_info statement would get an additional parameter, the label for the corresponding PORT functor. Also I would have to make sure that the modpaths are connected correctly when -gspecify is enabled.

At this point I may need some help from you:

Since I don't have much experience with the code generator yet, could you tell me if it is generally possible to output the code like this, or is there something I haven't thought of yet? I am open to any suggestion regarding this implementation.

Performance Implications

What are the performance implications for designs that do not use SDF INTERCONNECT?

For each module instance there are now PORT functors for the inputs and outputs. In a normal simulation, these functors would just pass values on and do nothing else. I think the performance impact would be small, but that needs to be measured with a benchmark.

I would suggest implementing the feature first and then evaluating the performance impact. If it is high, then interconnect support could be disabled with a command line flag. If it is low, it might be worth leaving it enabled by default.

That's it for now!

PS: Reminder to submit the midterm evaluation

[caryr]

@steveicarus any thoughts?

I will try to look at this in more detail soon.

Evaluation completed.

[mole99]

For your information, I have now handcrafted a VVP source file with three of the buffers in parallel to include vvp_fun_ports for their inputs and outputs as described above. I have also updated VVP to use these ports to figure out where to insert the vvp_fun_intermodpaths.
The interconnection delays to all three buffers were annotated correctly, and it works well even with specify blocks enabled.

[caryr]

That's great initiative and something I have needed to do a few times. Steve is the original author of this part of Icarus and while I understand it enough to make general comments and to fix an occasional bug I would need to look at it in more details to give more definitive answers. Hopefully he can reply soon and I will look at this in more detail later.

[mole99]

Some updates from my side: I tried to find other solutions to add connection delays, but I still think adding functors to the inputs and outputs is a good option. But I realized that we don't necessarily need to create a new functor, a BUFT will work just as well.

And as it turned out there is already functionality in VVP to add a BUFT before inputs. The need_bufz_for_input_port() function will return true if nexus()->drivers_present() evaluates to true. For now I have set it to always return true, so that BUFTs will always be created before inputs. This works well, with the only difference from my proposal being that the BUFTs are in the scope of the parent.

With this change, I am able to annotate the interconnect delays for the example with the three parallel buffers.
Not every situation is annotated correctly, there are some issues that I am still trying to solve. Therefore, I'm writing several smaller tests now to show what works and what doesn't. Later I want to try it on a larger design created with OpenLane.

[caryr]

I've been looking at the code and trying to think of alternatives. I don't see an obvious alternative. The most obvious enhancement is to only add the extra drivers when they could possibly be needed. Specifically when it is possible that a $sdf_annotate() will be called. I think if you look for a $sdf_annotate() call early enough in the compiler and then change the functionality of driver_present() for modules when an $sdf_annotate() is present this will give you the functionality you need? One further step would be to restrict this to the actual circuit hierarchy, but that may not always be available during the compile. Think run time values passed using plus arguments. Though if the value is given as a constant that could be used to further restrict the additional drivers.

Have you had time to look at the average change in the time needed to run the test programs if this functionality change is enabled or some other larger circuit simulation and how it is impacted by the change? Knowing the percentage change impact would be useful information.

I was hoping @steveicarus would have replied, but I don't think that will happen in a timely manner so we need to just make our best effort and deal with any suggestios if/when they come in.

[mole99]

This is a good way to automatically enable interconnect delays. Maybe by setting a flag during parsing when a call to $sdf_annotate() is detected. I'll look into this once the feature is working properly.

I haven't run any benchmarks to compare the runtime loss yet, but I will do that this week at the latest. I am in the process of adding references to the associated buffers in the .port_info directive. While working on this, I ran into a problem when I moved the input/output buffers into the scope of the module. I'll take a closer look soon, but maybe you have an idea what might be the case here: #985

[mole99]

I have benchmarked an RTL simulation of a RISC-V CPU with and without input / output buffers. The flag to enable the input / output buffers for interconnection delays is named -ginterconnect.

Results for -gno-interconnect:

Completed simulation.
soc/tb/icebreaker_top_tb.sv:64: $finish called at 65806736000 (1ps)
===> multitime results
1: make sim-icebreaker
            Mean        Std.Dev.    Min         Median      Max
real        73.810      1.063       72.095      73.771      75.648      
user        73.528      1.020       71.827      73.527      75.177      
sys         0.258       0.064       0.204       0.234       0.424

Results for -ginterconnect:

Completed simulation.
soc/tb/icebreaker_top_tb.sv:64: $finish called at 65806736000 (1ps)
===> multitime results
1: make sim-icebreaker
            Mean        Std.Dev.    Min         Median      Max
real        81.279      1.174       78.771      81.760      82.547      
user        81.044      1.172       78.526      81.573      82.159      
sys         0.211       0.023       0.176       0.214       0.245

So there seems to be a notable overhead of around 10% runtime. Consequently -ginterconnect should not be enabled by default.

Btw. it seems I have found the root cause for issue #985.

[caryr]

I assume this is the version that is making the change for all modules so there may be room for some improvement. Also are there any cases where there are now double buffers that could be optimized away? I will admit I was not expecting the performance degradation to be quite as high, but you can't argue with measurements :).

Good job in your sleuthing to root cause #985. That sounds like a subtlety that would exist in the code. Can the building code be changed to insert at the head for the src paths and at the tail for everything else? This is changing the core part of the simulator so we need to be mindful of both persistent compute and memory overhead for any change once the build stage has completed.

[mole99]

Yes exactly, all modules now contain input and output buffers because it is not possible to say which interconnects will be annotated. I am not sure about double buffers. I think they should get optimized away, since can_elide_bufz is only skipped for input and output buffers and not for any bufz.

I too didn't expect such a hit in performance, considering that RTL simulations have relatively few module instances in comparison to GL.

Thanks, it took me quite while to find the root cause :)
I will take a look at the code for how vvp_net_ts are connected to each other and how this can be changed for a net with a vvp_fun_modpath_src functor.

[mole99]

@caryr, I managed to fix the remaining annotation issues in #973
This means the designs in my test repository are simulated correctly: https://github.com/mole99/interconnect-tests
I also added three tests to the iverilog test suite to exercise the SDF INTERCONNECT feature and to prevent future regressions.

PR #973 is therefore ready for review and awaits your comments.

There are still some improvements to be added to the SDF INTERCONNECT feature, namely support for input/output vectors, but I would like to add this in a follow-up PR so that #973 doesn't accumulate too much code and functionality, making it much harder to review and maintain.

I will take a break from coding next week so that I can continue motivated and re-energized the week after.
I am well on schedule to complete the GSoC project sucessfully, but the end is fast approaching and we need to think to wrap it up properly.

Once SDF INTERCONNECT is in good shape, I would like to invest some of the remaining time in timing checks. They are essential for timing simulations, because nobody wants to check timing in waveforms by hand ;)

[mole99]

With the latest PR #994, it is now possible to annotate interconnection delays for input/output vectors.

I will continue my work by looking at implementing the $width timing check used in the SKY130 PDK.

[mole99]

Moving the discussion about SDF messages from #994 to here:

Yes, I too think a cleanup of the SDF messages is due. Let's check if we agree:

• Error: Fatal, ends the current annotation of IOPATH, INTERCONNECT, TIMINGCHECK. (Should it also stop the simulator?)
• Warning: Minor issue but annotation should continue if possible. (Unsupported/ignored constructs, multiple matching ports, etc.)
• Info: Useful information for the user and for debugging.

I would also propose to enable warnings by default. The user may not be aware that warnings need to be enabled and this could lead to seemingly correct simulations.

• Error: Always enabled
• Warning: Enabled by default, disabled via -sdf-no-warn
• Info: Disabled by default, enabled via -sdf-info

-sdf-verbose would then be removed

We could have a fatal category that stops the simulator and an error category that aborts the annotation. Neither could be disabled. Add to your existing or start a new discussion so others can comment.

[mole99]

I've been thinking a bit about what categories we should have, and I think the current ones should suffice. I would sort the current messages as follows:

• SDF ERROR: Fatal issues where the annotation can't be continued (Uses vpi_control(vpiFinish, 1); to exit the simulator)

  • Parse errors
  • Wrong arguments
  • SDF file cannot be read

• SDF WARNING: Enabled by default, skips the current annotation and continues with the next cell/interconnection

  • Unsupported features
  • Cell not found
  • Cell is not a module
  • Cell is of wrong type
  • Could not match modpath
  • Interconnect not found
  • Found multiple matching ports for interconnect
  • Bit of port not found

• SDF INFO: Useful information

  • SDF header
  • For each cell/interconnection

Let's leave this open for a bit to think about while I work on the timing check.

[mole99]

The final project report has been published here: GSoC23 — Final Project Report