verilog-to-routing / vtr-verilog-to-routing

Verilog to Routing -- Open Source CAD Flow for FPGA Research
https://verilogtorouting.org
Other
1k stars 388 forks source link

Unreachable code in vpr/src/route/connection_router.cpp #2570

Closed mchuahua closed 3 months ago

mchuahua commented 4 months ago

Expected Behaviour

Hi, in connection_router.cpp, line 119-122 has unreachable code.

Current Behaviour

I believe line 116 should return the correct tuple instead, which is reachable code.

Possible Solution

Remove lines 119-122 because line 124 returns what is expected when cheapest is found. I may be wrong though, which is why I didn't do a pr.

Steps to Reproduce

in connection_router.cpp, lines 100-122 below:

   if (cheapest == nullptr) {
        // No path found within the current bounding box.
        //
        // If the bounding box is already max size, just fail
        if (bounding_box.xmin == 0
            && bounding_box.ymin == 0
            && bounding_box.xmax == (int)(grid_.width() - 1)
            && bounding_box.ymax == (int)(grid_.height() - 1)
            && bounding_box.layer_min == 0
            && bounding_box.layer_max == (int)(grid_.get_num_layers() - 1)) {
            VTR_LOG("%s\n", describe_unrouteable_connection(source_node, sink_node, is_flat_).c_str());
            return std::make_tuple(false, nullptr);
        }

        // Otherwise, leave unrouted and bubble up a signal to retry this net with a full-device bounding box
        VTR_LOG_WARN("No routing path for connection to sink_rr %d, leaving unrouted to retry later\n", sink_node);
        return std::make_tuple(true, nullptr);
    }

    if (cheapest == nullptr) {
        VTR_LOG("%s\n", describe_unrouteable_connection(source_node, sink_node, is_flat_).c_str());
        return std::make_tuple(false, nullptr);
    }

Context

Your Environment

AlexandreSinger commented 3 months ago

@mchuahua Thank you for flagging this issue. Looking into the history of this file, this if statement that you flagged was used to catch the case when the router failed after retrying with a max bounding box. This was made more explicit in the previous if statement; however, the old if statement remained.

I have raised PR #2604 which removes this second if statement which should not have been left behind.

vaughnbetz commented 3 months ago

Thanks! Fixed and closing.