Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter uniquification style (+ other small improvements) #8

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

grg
Copy link
Collaborator

@grg grg commented Jan 19, 2025

Only review ea33d5b3 .. 203e2ea1. The other commits are in different PRs.

Uniquification style

Primary change in this PR is the addition of a new parameter uniquification style. Instead of making modules unique by adding the suffix _unq<number>, instead modules are uniquified by adding abbreviated versions of their parameters (name + value).

The uniquification suffix is formed concatenating the abbreviated parameter name plus its suffix (e.g., if MY_PARAM is set to 2, then the suffix _MP_2 is appended to the input module name) for each parameter passed specified in the generate call.

The abbreviation algorithm attempts to take the first letter of each word in the parameter name. In situations where it detects multiple parameters would produce the same abbreviation, it increases the number of letters included. For example;

  • A single parameter MY_PARAM is abbreviated to MP.
  • Two parameters MY_PARAM and MIN_POS both produce MP if we only take the first letter of each word. Instead, the algo produces MYP and MIP respectively for the two parameters.

Motivation:
This new uniquification style makes it much easier to identify which instance of a given module corresponds to a particular parameter set.

Consider a memory wrapper that's frequently used in a project and which takes a ROWS and WIDTH parameter. It's hard to tell from the filename mem_wrapper_unq15 what the corresponding memory size is, but it's easy to identify from mem_wrapper_R_1024_W_137.

Limitations:
I haven't yet added code to handle "complex" parameter values. When values are simple types (strings, integers), then the naming is as you would expect. However, if a value is complex (e.g., a list, a reference), then the filename will end up with the perl representation of that complex value, e.g., _MP_ARRAY(0x12345678).

Default behavior
I've made this new uniquification style the default because of its usefulness. You can still select the old uniquification style through command-line parameters.

Let me know if you want me to hold off on this for now.

Other changes

  • Instance paths are cached to speed up calls to get_instance_path. Without this, get_instance_path calls can greatly add to execution time in large designs.
  • Inputlist files support a richer set of features. In addition to specifying a list of files, then can now also specify -input, -inputlist, -incpath, and -srcpath values equivalent to those specified on the commandline.
  • Added generate_w_name function to generate a module with a specific name.
  • Avoid parsing the same file multiple times.

grg added 14 commits January 19, 2025 15:17
Update the license info printed when the program is executed to match the
text in the README.md file.
Use `#!/usr/bin/env perl` instead of `#!/usr/bin/perl` to allow
overriding the default version of perl installed on the system.
Remove the Genesis binaries with compiled-in modules. Want changes to
PerlLibs/Genesis2 to impact the program.
Cache the instance path to speed up calls to get_instance_path. This function
is called frequently, so cache the result to avoid repeated string
concatenation.
Adds a new module uniquification style that incorporates the parameter
names and values into the filename, instead of a sequentially increasing
number (the traditional approach). This is useful when using Genesis as
part of an ASIC flow, instead of just as a tool for design-space
exploration. By having the paramter names/values as part of the file
name, it makes it easier to identify which instance of a module
corresponds to which param set.

The parameter uniquification style is the new default.

Parameter names are abbreviated before being included in the file name.
The abbreviation logic prefers to take only the first letter from each
word, together with full numbers. However, if two parameters produce the
same abbreviation, then the abbrev alg will increase the number of
letters in order to create a unique abbreviation.

For example;
 - A single parameter "MY_PARAM" is abbreviated to "MP".
 - Two parameters "MY_PARAM" and "MIN_POS" both produce "MP" if we only
   take the first letter of each word. Instead, the algo will produce
   "MYP" and "MIP" respectively for the two parameters.

The values are converted to strings with a little sanitization (periods
and quotes are converted to underscores). This works well for simple
data types (e.g., strings, numbers), but produces unusual results for
complex data types and references.
Add description of the -unqstyle parameter.
Allow inputlist files to include these commands, in addition to files:
 -input
 -inputlist
 -incpath
 -srcpath
Add generate_w_name to generate a module with a specific name.
Add parse_unprocessed_file to parse a file if it hasn't already been
parsed. Based on parse_files.
Add a version of find_file that does _not_ error out when a file is not
found. The function returns undef if the file is not found.
Add add_suffix function that attempts to find the suffix to add to a
base module name.

Call add_suffix and parse_unprocess_file from load_base_module.
Extract common code from parse_files and parse_unprocessed_file.
@grg grg force-pushed the gleng/param_uniquify branch from 203e2ea to 4c6b7f4 Compare January 19, 2025 22:18
@grg grg marked this pull request as ready for review January 19, 2025 22:18
@grg grg requested a review from steveri January 19, 2025 22:18
@steveri
Copy link
Contributor

steveri commented Jan 22, 2025

Let me know if you want me to hold off on this for now.

No need to hold off yet BUT of course I will want to do extensive testing.
In particular I will be running the full suite of aha tests using these new changes.
The test suite itself takes upwards of 24-36 hours IIRC, plus there will be setup time and such.

I hope it works, because I feel like this is a great improvement. Thanks for doing this!

Kalhan, Yuchen, please forward to anyone in your group that you think may be interested...
@kalhankoul96
@yuchen-mai

@steveri
Copy link
Contributor

steveri commented Jan 22, 2025

Hm looks like maybe something is broken here.

This version of Genesis seems to find input files BUT ONLY if they are in the top level of the directory hierarchy.

This is the short version of what I see:

% ls -l jtag_ifc.svp global_controller/sim/genesis/jtag_ifc.svp
-rw-r--r-- 1 root root 867 Jan 12 23:43 global_controller/sim/genesis/jtag_ifc.svp
-rw-r--r-- 1 root root 867 Jan 22 10:05 jtag_ifc.svp

% Genesis2.pl -parse -top jtag_ifc -input jtag_ifc.svp
<succeeds with no complaints>

% Genesis2.pl -parse -top jtag_ifc -input global_controller/sim/genesis/jtag_ifc.svp
ERROR    ERROR    ERROR    ERROR    ERROR    ERROR    ERROR    ERROR
Genesis2::Manager ERROR in Line  of File global_controller/sim/genesis/jtag_ifc.svp
ERROR Message:

        Genesis2::Manager->find_file: Can not find file global_controller/sim/genesis/jtag_ifc.svp 
         Search Path:                                                                   

Exiting Genesis2 due to fatal error... bye bye... 

% cat jtag_ifc.svp
/*=============================================================================
** Module: jtag_ifc
** Description:
**              jtag interface
** Author: Taeyoung Kong
** Change history: 04/12/2019 - Implement first version of interface
**===========================================================================*/

interface `$self->get_module_name()`();

    logic                                tck;
    logic                                tdi;
    logic                                tms;
    logic                                trst_n;
    logic                                tdo;

    modport dut (
        input  tck,
        input  tdi,
        input  tms,
        input  trst_n,
        output tdo
    );

    modport test (
        output tck,
        output tdi,
        output tms,
        output trst_n,
        input tdo
    );

endinterface

Let me know if you want the longer version, with my complete setup from beginning to end etc. ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants