Placement delay lookup can not be readback

[tangxifan]

The placement delay lookup is a useful binary file which saves computing timing for a fixed device.

Expected Behaviour

Similar to the routing resource graph (RRGraph), its usage is expected to be simple.
Users can enable the option, e.g., --write_placement_delay_lookup pdlylkup.bin, to output the data into a binary file.
When in later VPR runs, users can enable the option, e.g., --read_placement_delay_lookup pdlylkup.bin, to readback the data from the binary file.

Current Behaviour

The binary file written out cannot be read back. A segmentation fault is shown.

Possible Solution

Should fix this bug if verified. Readback is the simplest case.
Expect to have a dedicated testcase in strong regression tests, to avoid the bug to occur later

Steps to Reproduce

1. Download the latest VPR
2. Compile with make all -j32
3. python3 run_quick_test.py
4. cd vtr_flow/tasks/regression_tests/vtr_reg_basic/basic_timing/run001/k6_N10_mem32K_40nm.xml/ch_intrinsics.v/common
5. <YOUR_VTR_ROOT>/vtr-verilog-to-routing/vpr/vpr k6_N10_mem32K_40nm.xml ch_intrinsics --circuit_file ch_intrinsics.pre-vpr.blif --route_chan_width 60 --write_placement_delay_lookup pdlylkup.bin
6. <YOUR_VTR_ROOT>/vtr-verilog-to-routing/vpr/vpr k6_N10_mem32K_40nm.xml ch_intrinsics --circuit_file ch_intrinsics.pre-vpr.blif --route_chan_width 60 --read_placement_delay_lookup pdlylkup.bin
7. You should see the same error as I do

Context

Your Environment

• VTR revision used: ce706d5

• Operating System and version: Ubuntu 22.04

• Compiler version: GCC-12

[tangxifan]

@vaughnbetz I believe that this is a critical bug, as a show stopper to anyone who is following latest VTR.

[vaughnbetz]

@soheilshahrouz @amin1377 : perhaps this was caused by a recent refactoring? Could one of you take a look?

I imagine (hope!) this is covered in the nightly tests, but we unfortunately aren't running them in CI automatically right now. We should make a strong test that writes out an rr-graph, router lookahead and place delay matrix and then reads them all back in again, all on a small device.

[soheilshahrouz]

8944b98 changed the defualt placement delay model to simple.

SimpleDelayModel has no implementation for read() and write() methods. So the file is not even written. When, vpr is run with --read_placement_delay_lookup, the SimpleDelayModel is not initialized as its read() method does nothing.

vtr-verilog-to-routing/vpr/src/place/place_delay_model.h

Lines 246 to 247 in ce706d5

	void read(const std::string& /*file*/) override {}
	void write(const std::string& /*file*/) const override {}

So the binary file is not even written. When, vpr is run with --read_placement_delay_lookup, the SimpleDelayModel is not initialized as its read() method does nothing. The first time delay() is called, we access an uninitialized delays_ matrix, causing a segmentation fault.

vtr-verilog-to-routing/vpr/src/place/place_delay_model.cpp

Lines 155 to 161 in ce706d5

	float SimpleDelayModel::delay(const t_physical_tile_loc& from_loc, int /*from_pin*/, const t_physical_tile_loc& to_loc, int /*to_pin*/) const {
	int delta_x = std::abs(from_loc.x - to_loc.x);
	int delta_y = std::abs(from_loc.y - to_loc.y);
	int from_tile_idx = g_vpr_ctx.device().grid.get_physical_type(from_loc)->index;
	return delays_[from_tile_idx][from_loc.layer_num][to_loc.layer_num][delta_x][delta_y];
	}

[vaughnbetz]

Thanks Soheil. The simple option presumably builds the placement delay matrix from the router lookahead, which should be pretty fast. Can you time the place_delay_matrix build time on a big circuit? If it is very fast, one option would be to simply ignore (with a warning) the read & write options with this type of place_delay matrix (as reading the router_lookahead would achieve nearly the same result).

Or we could write the read and write methods for this placer delay matrix (or re-use existing ones if that would work). That is probably the cleanest of all so if it's straightforward that's probably best.

[tangxifan]

@vaughnbetz @soheilshahrouz Thanks for the insights. If I specify the place_delay_model to be delta which was the default model before 8944b98 is merged, the place_delay_matrix I/O should work?

I am trying to bypass the issue for now

[vaughnbetz]

Yes, based on the conversation today that should work.

[tangxifan]

Thanks @vaughnbetz for the prompt help! Appreciate it.