advice for dependency analysis?

[fpgauserdude]

I am exploring the possibility of using slang for a python-based HDL build system (I am currently using a different parser, but slang seems to have some advantages over my current solution). I need to extract the dependencies of an arbitrary SystemVerilog file. To do this correctly requires preprocessing and parsing the SV code. Is there a way use use slang/pyslang that you would recommend for this application? Do I need to parse the code and then walk the AST? I found the "runPreprocessor" method in the Driver.py namespace, and I am experimenting with that. But I would very much appreciate any advice you might have.

Note that I looked at the "-Mall" option of the slang executable, but the resulting output does not find most of the dependencies for my test case. Specifically, it misses package, interface and module dependencies. I would be happy to share my test case if it would be helpful.

[MikePopoloski]

I would expect the existing dependency tracking stuff to do what you're saying, so perhaps there is a bug to be fixed there.

The existing logic looks at references in the parse tree, which you could replicate in pyslang easily enough. Using the AST should be unnecessary unless you want to figure out dependencies in some more specific way based on the results of elaboration / full name resolution.

[fpgauserdude]

Thanks for your response.

The existing logic looks at references in the parse tree, which you could replicate in pyslang easily enough. Using the AST should be unnecessary...

I don't follow you very well. Isn't the AST the same as the parse tree? I will do some experimentation.

If you are interested, here is my test case (just a modified version of code from an online library); it should find the following dependencies:
- import_pkg
- param_pkg
- test_if
- test_pkg
- bdy_Pkg
- bdy_pkg2
- bsg_async_ptr_gray
- bsg_gray_to_binary

`include "bsg_defines.v"
package hello_pkg;
  localparam CONST1 = 8;
  typedef int type_i[0: CONST1-1];
  class heres_a_class;
    int member;
    function new();
      member = 0;
    endfunction
  endclass
endpackage // hello_pkg

module hello_module();
  localparam TEST = hello_pkg::CONST1;
endmodule

module bsg_async_credit_counter
  // this is apparently valid!
  import import_pkg::*;
 #(parameter max_tokens_p
   , parameter lg_credit_to_token_decimation_p
   , parameter param_pkg::count_negedge_p test = 0
   , parameter extra_margin_p = 0
   , parameter check_excess_credits_p = 1
   , parameter start_full_p = 1
   , parameter use_async_w_reset_p = 0)
    (
      input w_clk_i
    , input wire w_inc_token_i
    , test_if intf1
    , test_pkg::test_type input_struct
    , input w_reset_i
    , input r_clk_i
    , input r_reset_i
    , input r_dec_credit_i
    , input r_infinite_credits_i  // basically suppress this module
    , output logic r_credits_avail_o
    );

  hello_module hello();


   // $clog2(x) is how many bits are required to represent
   // x unique values. we need to represent 0..max_credits_p = max_credits_p+1 values.

   localparam r_counter_width_lp = extra_margin_p+$clog2(max_tokens_p+1) + lg_credit_to_token_decimation_p;
   localparam w_counter_width_lp = extra_margin_p+$clog2(max_tokens_p+1);

   logic [r_counter_width_lp-1:0] r_counter_r;
   logic [w_counter_width_lp-1:0] w_counter_gray_r, w_counter_gray_r_rsync, w_counter_binary_r_rsync;


   localparam                     bdy_Pkg::testing param_t = '1;
   bdy_pkg2::testother struct_type;

   always @(posedge r_clk_i)
     if (r_reset_i)
       // fixme? not sure this constant will always do as expected
       r_counter_r       <= { -max_tokens_p * start_full_p, { lg_credit_to_token_decimation_p  {1'b0} } };
     else
       r_counter_r       <= r_counter_r + r_dec_credit_i;

   // *********** this is basically an async_ptr: begin factor

   bsg_async_ptr_gray #(.lg_size_p(w_counter_width_lp)
                        ,.use_negedge_for_launch_p(count_negedge_p)
                        ,.use_async_reset_p(use_async_w_reset_p)) bapg
   (.w_clk_i(w_clk_i)
    ,.w_reset_i(w_reset_i)
    ,.w_inc_i(w_inc_token_i)
    ,.r_clk_i(r_clk_i)
    ,.w_ptr_binary_r_o() // we don't care about the binary version of the ptr on w side
    ,.w_ptr_gray_r_o(w_counter_gray_r)             // synchronized with w clock domain
    ,.w_ptr_gray_r_rsync_o(w_counter_gray_r_rsync) // synchronized with r clock domain
    );

/*
    previously, we converted w_counter to binary, appended lg_credit_to_token_decimation 1'b0's and compared them
    but instead, we convert the other way now.
    assign r_credits_avail_o = r_infinite_credits_i | ~(w_counter_binary_r_rsync_padded == r_counter_r);
*/

   wire [w_counter_width_lp-1:0] r_counter_r_hi_bits         =   r_counter_r[lg_credit_to_token_decimation_p+:w_counter_width_lp];
   wire                          r_counter_r_lo_bits_nonzero;
   if (lg_credit_to_token_decimation_p == 0)
     assign r_counter_r_lo_bits_nonzero = 1'b0;
   else
     assign r_counter_r_lo_bits_nonzero = | r_counter_r[0+:lg_credit_to_token_decimation_p];
   wire [w_counter_width_lp-1:0] r_counter_r_hi_bits_gray    = (r_counter_r_hi_bits >> 1) ^ r_counter_r_hi_bits;

   assign r_credits_avail_o = r_infinite_credits_i | r_counter_r_lo_bits_nonzero | (r_counter_r_hi_bits_gray != w_counter_gray_r_rsync);

`ifdef TEST_DEFINE
   test_module  bapg();

`else // !`ifdef TEST_DEFINE
     test_module2 #(.lg_size_p(w_counter_width_lp)
                        ,.use_negedge_for_launch_p(count_negedge_p)
                        ,.use_async_reset_p(use_async_w_reset_p)) bapg
   (.w_clk_i(w_clk_i)
    ,.w_reset_i(w_reset_i)
    ,.w_inc_i(w_inc_token_i)
    ,.r_clk_i(r_clk_i)
    ,.w_ptr_binary_r_o() // we don't care about the binary version of the ptr on w side
    ,.w_ptr_gray_r_o(w_counter_gray_r)             // synchronized with w clock domain
    ,.w_ptr_gray_r_rsync_o(w_counter_gray_r_rsync) // synchronized with r clock domain
    );
`endif



   // ***************************************
   //  for debug
   //
   //


   // synopsys translate_off

   bsg_gray_to_binary #(.width_p(w_counter_width_lp)) bsg_g2b
     (.gray_i(w_counter_gray_r_rsync)
      ,.binary_o(w_counter_binary_r_rsync)
      );

   wire [r_counter_width_lp-1:0]  w_counter_binary_r_rsync_padded = { w_counter_binary_r_rsync, { lg_credit_to_token_decimation_p {1'b0 } }};

   wire  [r_counter_width_lp-1:0] r_free_credits =  w_counter_binary_r_rsync_padded - r_counter_r;

   logic [r_counter_width_lp-1:0] r_free_credits_r;

   always @(posedge r_clk_i)
     r_free_credits_r <= r_free_credits;

   if (check_excess_credits_p)
     always @(r_free_credits_r)
       assert(r_reset_i
              |  r_infinite_credits_i
              | (r_free_credits_r <= (max_tokens_p << lg_credit_to_token_decimation_p)))
              else $error("too many credits in credit counter %d (> %3d)"
                          , r_free_credits_r
                          , max_tokens_p << lg_credit_to_token_decimation_p
                          );

   always @(negedge r_clk_i)
       assert (!(r_dec_credit_i===1 && r_credits_avail_o===0))
              else $error("decrementing empty credit counter");

   // synopsys translate_on

   //
   // end debug
   //
   // ****************************************

endmodule

`BSG_ABSTRACT_MODULE(bsg_async_credit_counter)```

[MikePopoloski]

The parse tree is the concrete syntax tree; it represents exactly what was written in the code. The AST is abstract, names are resolved, module instances are expanded, types are resolved, etc.

Your example isn't very helpful as-is; the built-in dependency file stuff prints out which files are needed to include to get all of the referenced symbols. It seems like you don't want files, you want symbol names themselves, which is actually slightly simpler but doesn't exit as a built-in command. You can poke around in the getSortedDependencies function from Driver.cpp to see what it's doing.

[fpgauserdude]

Yes, I need the symbol names in order to automatically resolve the project structure and compile order.

I am a little confused how the built-in feature is useful. The output file list does not include the referenced symbols (am I not referencing "import_pkg" by instancing it?). It lists the included file(s), which I suppose might be useful for automatically determining the necessary include directories, but typically a build system requires the user to specify those directories in advance. We probably have a difference in our terminology, but I would like to understand how you envision someone using the -Mall feature.

Thanks again. I'll take a look at Driver.cpp.

[MikePopoloski]

It doesn't just list included files, it lists source files as well. Those aren't necessarily specified manually on the command line, they can be pulled in from library maps or via the automatic search feature with --libdir. Also if you have shared code that lists lots of potentially unnecessary source files (possibly only determined after resolving ifdefs with the preprocessor) the --depfile-trim option will remove any of them aren't needed. The resulting dependency file represents the exact set of files you need to build the design, which you could pass to other tools that are less flexible for whatever reason.

[fpgauserdude]

I see. That is useful, and is something I could utilize. Right now my build system uses all the files contained in the input include directories as dependencies. In other words, if any file changes in the directory supplied to the preprocessor, all dependencies using that directory will be recompiled even if that file is not actually used. Your solution is more efficient and more correct than what I am currently doing.

Thanks for the explanation.

[fpgauserdude]

I looked at the getSortedDependencies method in Driver.cpp. Are you suggesting that this functionality could be duplicated using pyslang?

I attempted this:

"""Reads a list of files as command line arguments and parses them using the slang driver.

    After compilation/elaboration, any diagnostics (e.g., syntax errors) are reported to the console.
    Writes to both stderr and stdout.
    """
    # Create a slang driver with default command line arguments
    driver = Driver()
    driver.addStandardArgs()

    # Parse command line arguments
    args = " ".join(sys.argv)
    if not driver.parseCommandLine(args, CommandLineOptions()):
        return

    # Process options and parse all provided sources
    if not driver.processOptions() or not driver.parseAllSources():
        return

    # Perform elaboration and report all diagnostics
    #driver.runFullCompilation(quiet=False)
    # I have also looked at the driver.syntaxTrees list
    comp = driver.createCompilation()
    driver.runAnalysis(comp)
    trees = comp.getSyntaxTrees()

but the resulting "trees" data structure does not appear to have the data exposed to Python that getSortedDependencies is using. Am I wrong about that? Maybe I am not running the compilation process correctly? For example, there is no "getMetadata()" method exposed to the elements of the "trees" list returned in the code above.

I suppose I could attempt to write a C++ utility that would run getSortedDependencies and print the result. It looks like I could use code inthe "optionallyWriteDepFiles()" method as a model.

But I would prefer to do this using only python code.

[MikePopoloski]

Ah yeah, it looks like the ParserMetadata class and related types are not exposed in the bindings currently. It's not particularly hard to add if you want to take a stab at it.

[fpgauserdude]

I have built a new C++ utility based on the code in getSortedDependencies. I am not familiar enough with exposing C++ classes to python to go the python route. The utility almost works. But it is not finding the "test_if" interface reference in the port map of the example I provided above. Is this because SV tools typically wait until elaboration to find data types like this? Do you have any suggestions?

When I write out the CST, I see the following:

                  {
                    "kind": "ImplicitAnsiPort",
                    "header": {
                      "kind": "VariablePortHeader",
                      "dataType": {
                        "kind": "NamedType",
                        "name": {
                          "kind": "IdentifierName",
                          "identifier": {
                            "kind": "Identifier",
                            "text": "test_if",
                            "trivia": [
                              {
                                "kind": "Whitespace",
                                "text": " "
                              }
                            ]
                          }
                        }
                      }
                    },
                    "declarator": {
                      "kind": "Declarator",
                      "name": {
                        "kind": "Identifier",
                        "text": "intf1",
                        "trivia": [
                          {
                            "kind": "Whitespace",
                            "text": " "
                          }
                        ]
                      }
                    }
                  },

Is there any existing method that will help me find this, or must I walk the tree myself? It might be tricky to distinguish this type from other types that do not represent external dependencies.

[MikePopoloski]

test_if is not unambiguously an interface reference. You don't know until you do full elaboration and name resolution whether it's an interface or a data type, possibly one imported via a prior wildcard package import. If you know for your specific use case that you want all named types to be included in the list of references then it's simple enough to walk the CST and find them for inclusion, but the existing code does not do that.

[fpgauserdude]

SystemVerilog is a crazy language. I have struggled with this in the past and probably have some flaws in my solution.

I can probably get away with allowing this dependency to be missing from the module that uses the interface in its port map (because presumably, the interface will be instanced later and that will allow the interface to be compiled into the library to be found later at elaboration time). But this is technically incorrect. I can envision a situation where someone changes the interface source file, and because the dependency tree is incorrect, the module using the interface might not be flagged for recompilation, leading to strange errors.

So it looks like I do need to walk the tree and look for "NamedType" fields (probably only inside port maps). But disambiguating types at this stage is really hard. Maybe the build system lists named types and compares them against a list of known interfaces? That's a pretty hackish solution. Maybe I'm stuck relying on some kind of pragma that forces the dependency. That is also suboptimal since I am trying to build a system that can automate builds of arbitrary SV code.

If you have any thoughts, I would be interested to hear them.

[MikePopoloski]

The systems I've seen all rely on the thing you mentioned, which is that some other module somewhere is going to instantiate this interface and so you'll find the dependency that way. Otherwise, if your input is truly arbitrary SV then there's literally no way to disambiguate without having all of the files in the design. Like you can imagine a scenario like this:

file1.sv:

module m import p::*; (foo f);
endmodule

file2.sv:

package p;
  `ifdef SOME_DEF
    typedef logic foo;
  `endif
endpackage

file3.sv:

interface foo;
endinterface

Three separate files. If SOME_DEF is defined, your module has a logic wire port. If not, it has an interface port.

[fpgauserdude]

I have a working prototype.

I congratulate you on building an excellent SystemVerilog parser and suite of tools. Between the ability to "keep going" in the presence of syntax errors, providing dependent files used by the preprocessor, and being written in C++ (with a sane build system), this tool is better than anything else I am aware of. So far it hasn't had any issues (even with some very unusual SystemVerilog code that gives other parsers lots of problems).

I hope a future is not far off where companies like Verific, Siemens, Cadence, etc. no longer have a license to steal.

[MikePopoloski]

Thanks for the kind words, glad you were able to get something working.

[fpgauserdude]

One more question: Is there a built in way to parse data from stdin, or would I need to add that? Other tools use a file name like "-" to indicate that stdin should be used.

I suppose my need for this is because I do not have python bindings, so maybe that is an option too.

[MikePopoloski]

Yes, "-" works to refer to stdin / stdout in command line args.