Skip to content

Reorganizing vqm2blif source code #1850

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Sep 22, 2021
Merged

Reorganizing vqm2blif source code #1850

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d808e99
made changes so that the node removal function can be used to remove …
Srivat97 Sep 9, 2021
6a4f91f
added a seperate function to reorganize module node array after delet…
Srivat97 Sep 9, 2021
0f63c41
made the remove_node function accessible outside of cleanup.cpp and a…
Srivat97 Sep 9, 2021
5aabcdd
fixin redundant declaration of the remove_node function
Srivat97 Sep 10, 2021
f294690
added a test script that can be used to verify the vqm2blif program
Srivat97 Sep 15, 2021
84e12d8
added a suite of test benchmarks that can be used when verifying the …
Srivat97 Sep 15, 2021
f2b9e79
Improved readibility of the vqm2blif program test script output. Also…
Srivat97 Sep 15, 2021
9c35fc6
Added a new job to the github workflow to automatically test the vqm2…
Srivat97 Sep 15, 2021
e616752
added additional commenting to better describe the test script usage
Srivat97 Sep 16, 2021
25dfbd7
Merge branch 'master' into reorganizing_vqm2blif_source_code
vaughnbetz Sep 17, 2021
5f98b6b
removed unnecessary parameters from the reorganize_module_node_list f…
Srivat97 Sep 21, 2021
0bec813
removed unnecessary function parameters from the remove_one_lut_nodes…
Srivat97 Sep 21, 2021
0e7383c
added general comments to the functions in cleanup.h
Srivat97 Sep 21, 2021
37d2e42
updated vqm2blif test script to now allow input flags and provide usa…
Srivat97 Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,25 @@ jobs:
./.github/scripts/build.sh
./run_reg_test.py odin_reg_basic -show_failures -j2
VQM2BLIF:
name: 'VQM2BLIF Basic Tests'
runs-on: ubuntu-18.04
steps:

- uses: actions/setup-python@v2
with:
python-version: 3.6
- uses: actions/checkout@v2
- run: ./.github/scripts/install_dependencies.sh

- name: Test
env:
BUILD_TYPE: release
run: |
./.github/scripts/build.sh
./utils/vqm2blif/test/scripts/test_vqm2blif.sh
YOSYSODINII:
runs-on: ubuntu-18.04
Expand Down Expand Up @@ -256,6 +275,7 @@ jobs:
- Regression
- Sanitized
- ODINII
- VQM2BLIF
- YOSYSODINII
- Compatibility
runs-on: ubuntu-18.04
Expand Down
98 changes: 71 additions & 27 deletions utils/vqm2blif/src/base/cleanup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ void verify_netlist ( t_node** nodes, int num_nodes, busvec* buses, struct s_has
void print_all_nets ( busvec* buses, const char* filename );

bool is_feeder_onelut ( t_node* node );
void remove_node ( t_node* node, t_node** nodes, int original_num_nodes );

//============================================================================================
//============================================================================================
Expand Down Expand Up @@ -278,6 +277,9 @@ void remove_one_lut_nodes ( busvec* buses, struct s_hash** hash_table, t_node**
---->
|
---->
Change Log:
- Srivatsan Srinivasan, August 2021:
- Moved the incrementing of the "oneluts_elim" variable to this fuction from the "remove_node" function. The purpose of this change was to localise any vairable attached to removing one lut nodes within this function. Additionally, now the "remove_node" function is generalized and is not limited to be only used by "remove_one_lut_nodes".
*/
oneluts_elim = 0;

Expand Down Expand Up @@ -344,37 +346,17 @@ void remove_one_lut_nodes ( busvec* buses, struct s_hash** hash_table, t_node**

//Free the LUT
remove_node(source_node, nodes, original_num_nodes);

// increment the count of the number of one lut nodes we eliminiated
oneluts_elim++;

}
}
}
}

//Regorganize nodes array by filling in gaps with the last available elements in the array to save CPU time
int new_array_size = original_num_nodes - oneluts_elim;
int curr_node_index = 0;
int replacement_node_index = original_num_nodes - 1;
while (curr_node_index < replacement_node_index) {
if (nodes[curr_node_index] == NULL) {
if (nodes[replacement_node_index] != NULL) {
//Replace gap with node
nodes[curr_node_index] = nodes[replacement_node_index];
nodes[replacement_node_index] = NULL;
curr_node_index++;
}
replacement_node_index--;
} else {
curr_node_index++;
}
}
if (nodes[curr_node_index] == NULL) {
VTR_ASSERT(curr_node_index == new_array_size); //check array size
} else {
VTR_ASSERT(curr_node_index == new_array_size - 1); //check array size
}

//Update array bounds
module -> number_of_nodes = new_array_size;
// reorganize the node array after removing all the one lut nodes previously
reorganize_module_node_list(original_num_nodes, oneluts_elim, nodes, module);

//Reduce run-time by only verifying at the end
//verify_netlist (nodes, module->number_of_nodes, buses, hash_table);
Expand Down Expand Up @@ -829,6 +811,12 @@ bool is_feeder_onelut ( t_node* node ) {
void remove_node ( t_node* node, t_node** nodes, int original_num_nodes ) {
//Free node and assign it to NULL on the spot
//Array will be re-organized to fill in the gaps later
/*
Change Log:
- Srivatsan Srinivasan, August 2021:
- Orirignally, the "oneluts_elim" variable (which is used to keep track of the number of one lut nodes removed from the node list) was incremented here. This incrementation has been moved to the "remove_one_lut_nodes" function.
- This purpose of this change is that now this function is not limited to just be used by "remove_one_lut_nodes".
*/

VTR_ASSERT(node != NULL);
VTR_ASSERT(nodes != NULL);
Expand All @@ -848,8 +836,64 @@ void remove_node ( t_node* node, t_node** nodes, int original_num_nodes ) {
}

VTR_ASSERT(found);
oneluts_elim++;
}

//============================================================================================
//============================================================================================

void reorganize_module_node_list(int original_number_of_nodes, int number_of_nodes_eliminated, t_node** module_node_list, t_module* module)
{
/*
The purpose of this function is to regorganize the nodes array by filling in gaps with the last available elements in the array to save CPU time.

The node array provided to this function is expected to have elements deleted within. THis essentially creates gaps in the array. So we fill in the gaps of this array and ensure it is a continuous list of nodes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explain that the node array is module_node_list, and that deleted elements within it are NULL.
Do you need original_number_of_nodes, or would that be equal to module->number_of_nodes?
number_of_nodes_eliminated seems dangerous to pass in. It should be redundant with the number of NULL entries in the module_node_list, so it doesn't have to be passed. It isn't asserted to match the number of NULLs eliminated, so it is also a bit dangerous -- if it doesn't match then the module_node_list could be corrupted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been updated now. number_of_nodes and nodes have been removed. Additionally, the number_of_nodes_eliminated has also been removed as a parameter and we are now determining this within the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function "remove_one_lut_nodes" also had unnecessary parameters that could have been found through "module", so the function was also changed as well.


Please refer to the example below:

Inital Node array:
------ ------ ------ ------
|LUT 1| --> |LUT 2| --> --> |LUT 3| --> |LUT 4|
------ ------ ------ ------

After reorganization and filling gaps:

------ ------ ------ ------
|LUT 1| --> |LUT 2| --> |LUT 3| --> |LUT 4| -->
------ ------ ------ ------

Change Log:
- Srivatsan Srinivasan, August 2021:
- created this function to reorganize node arrays with gaps inside of them.
- Initially the feature provided by this function was embedded indide the "remove_one_lut_nodes" function. By creating a seperate function, we are now not restricted to only removing one-lut nodes.
- Now we can remove any types of nodes and then run this function to reorganize the node array.
*/
int new_array_size = original_number_of_nodes - number_of_nodes_eliminated;
int curr_node_index = 0;
int replacement_node_index = original_number_of_nodes - 1;
while (curr_node_index < replacement_node_index) {
if (module_node_list[curr_node_index] == NULL) {
if (module_node_list[replacement_node_index] != NULL) {
//Replace gap with node
module_node_list[curr_node_index] = module_node_list[replacement_node_index];
module_node_list[replacement_node_index] = NULL;
curr_node_index++;
}
replacement_node_index--;
} else {
curr_node_index++;
}
}
if (module_node_list[curr_node_index] == NULL) {
VTR_ASSERT(curr_node_index == new_array_size); //check array size
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// check array size is too low level a comment (doesn't really tell me what you're doing except that it is a check, which I can already tell from the assert.
Instead, comment the whole if statement to explain what you're checking, and why you need two cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entire block has now been removed. Originally, this assert block was used to verify whether the node list had the correct number of nodes after we had filled in all the gaps. The user originally supplied a parameter of the number of nodes within the array that were removed, using this a new array size was determined and this was compared to size of the node array after filling in all the gaps. But now, since we do not pass in the number of nodes removed, there is no need to perform this check as we now internally determine the node array size as we fix it.

} else {
VTR_ASSERT(curr_node_index == new_array_size - 1); //check array size
}

//Update array bounds
module -> number_of_nodes = new_array_size;

return;
}

//============================================================================================
//============================================================================================
4 changes: 4 additions & 0 deletions utils/vqm2blif/src/base/cleanup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ extern int buffers_elim, inverts_elim, oneluts_elim;

void netlist_cleanup (t_module* module);

void remove_node ( t_node* node, t_node** nodes, int original_num_nodes );

void reorganize_module_node_list(int original_number_of_nodes, int number_of_nodes_eliminated, t_node** module_node_list, t_module* module);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment what these functions do here (in the header) so people calling them just have to read the header. The comment should explain what each input to the function is, and what the function does (but doesn't have to explain how the function does it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments have been added now.


//============================================================================================
// STRUCTURES & TYPEDEFS
//============================================================================================
Expand Down
Loading