vtr-verilog-to-routing icon indicating copy to clipboard operation
vtr-verilog-to-routing copied to clipboard

Remove the use of global variable

Open baberali-rs opened this issue 2 years ago • 1 comments

Proposed Behaviour

Fully modularize the each engine of VTR.

Current Behaviour

There are global variables used in many functions.

Possible Solution

Remove the use of global variables in any function.

Context

baberali-rs avatar Sep 30 '21 06:09 baberali-rs

Let me give more background on this issue.

Motivation

Currently, VPR has an intensive use of global variables in low-level functions. For example,

https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/d58f993d07fe752dbe254132e739ce2d62abc9e5/vpr/src/route/rr_graph2.cpp#L570-L590

This has created many barriers for future technical developments

  • It is hard to modularize VPR engines or APIs. The use of global variables mean that there are hidden arguments for many functions. When extracting a VPR engine into a separated library, it is now very difficult because it is very hard to count the number of hidden variables.
  • The learning curve for junior developers is very steep. It is very frequent for junior engineer to make mistakes. The most common but fatal is to accidently modify any global variables, which causes a chain effect (other part of VPR breaks and the whole VPR shuts down).

image

Proposed Technical plan

The massive use of global variables is hard to be corrected in one or two pull requests. There are many functions impacted. And some functions are very low-level (may have 10+ levels in hierarchy). Modifying only one function may cause a large code change potentially. Therefore, I suggest to do it gradually through many small pull requests. And we start from one engine and rework one by one, until we finish all the engines.

  • Step 1: Routing resource graph/Router. We would like to focus on reworking all the source files in the vpr/src/route directory.
    • Our strategy is to rework some lowest level functions, such as describe_rr_nodes() as a first step
    • Then we start rework higher level functions, basically those functions which call the functions use describe_rr_nodes()
    • We keep following this bottom-up way until all the source files are reworked.
    • We may enrich code comments when needed during this refactoring process.
  • Step 2: Placer.
  • Step 3: GUI
  • Step 4: Timing analyzer
  • Step 5: Utils

We plan to have a slow but steady execution in Step 1, where we detailed the strategy and improve it once we see any challenges. Once step 1 is finished, step 2-5 can be executed in parallel.

tangxifan avatar Sep 30 '21 19:09 tangxifan