[Netlist] Potential Issue When Re-Creating Elements in Atom and Clustered Netlists

[AlexandreSinger]

When implementing another Netlist class based on the Atom and Clustered netlists, I noticed a bug in their create methods.

This bug occurs in a few places, but for example the create block method specifies that it creates a block or returns the ID if it already exists:

vtr-verilog-to-routing/vpr/src/base/atom_netlist.h

Lines 167 to 175 in 21d0150

	/**
	* @brief Create or return an existing block in the netlist
	*
	* @param name The unique name of the block
	* @param model The primitive type of the block
	* @param truth_table The single-output cover defining the block's logic function
	* The truth_table is optional and only relevant for LUTs (where it describes the logic function)
	* and Flip-Flops/latches (where it consists of a single entry defining the initial state).
	*/

However, in the implementation, the push_back method is used on the internalized data:

vtr-verilog-to-routing/vpr/src/base/atom_netlist.cpp

Lines 140 to 155 in 21d0150

	AtomBlockId AtomNetlist::create_block(const std::string& name, const t_model* model, const TruthTable& truth_table) {
	AtomBlockId blk_id = Netlist::create_block(name);
	//Initialize the data
	block_models_.push_back(model);
	block_truth_tables_.push_back(truth_table);
	//Check post-conditions: size
	VTR_ASSERT(validate_block_sizes());
	//Check post-conditions: values
	VTR_ASSERT(block_model(blk_id) == model);
	VTR_ASSERT(block_truth_table(blk_id) == truth_table);
	return blk_id;
	}

Netlist::create_block will return the ID of a block if one with that name already exists; but then there will be a mismatch with the internal data. This will cause a crash if a block is re-created into the netlist.

This can be simply fixed by changing push_back into insert. The performance should not be too different since the vtr::vector_map class treats insertions as a resize (similar to a push back).

This should be fixed for many of the create methods in both the Atom Netlist and the Clustered Netlist (or the documentation should be updated to reflect that re-creation of IDs is not supported).

[AlexandreSinger]

@ZohairZaidi This is a good coding issue that you may be able to help with. In the Atom and Clustered netlists, I noticed a bug with how they maintain their internal data. I was wondering if you wanted to take a crack at this issue? It should be relatively simple, it would just need to be tested through our CI to ensure nothing breaks.

This issue can be found in the "create_..." methods in the following two files:

• vpr/src/base/atom_netlist.cpp
• vpr/src/base/clustered_netlist.cpp

Feel free to let me know if you have any questions!

CC: @vaughnbetz

[ZohairZaidi]

Thanks @AlexandreSinger I just submitted a PR. Ran some local tests but will wait for the CI results as well. Please feel free to point out anything I may have done wrong or if you would like it a different way.