search

vaughnbetz / COFFE

38 stars 25 forks source link

Inconsistency with area and delay cost factors #32

Closed aman26kbm closed 3 years ago

aman26kbm commented 4 years ago

Hi Dr. Betz (@vaughnbetz ),

I see some discrepancy with the area and delay cost factors.

  1. In coffe/hardblock_functions.py file, it seems like there is a mistake. The following line lowest_cost = math.pow(float(total_area[0]), float(flow_settings['area_cost_exp'])) * math.pow(float(total_delay), float(flow_settings['**area_cost_exp**'])) should be: lowest_cost = math.pow(float(total_area[0]), float(flow_settings['area_cost_exp'])) * math.pow(float(total_delay), float(flow_settings['**delay_cost_exp**'])) This seems to be a copy-paste error, right? I can submit a PR for this.

  2. There are two places where area and delay cost factors can be specified. There are command line options for the tool overall:

    parser.add_argument('-a', '--area_opt_weight', type=int, default=1, help="area optimization weight")
parser.add_argument('-d', '--delay_opt_weight', type=int, default=1, help="delay optimization weight")

    And then there is a place to specify them for the hardblock standard cell flow in the hardblock settings file. Is that intentional? Is the idea that someone could want to have a different area/delay tradeoff inside the harblock, but a different tradeoff for the rest of the FPGA?

Thanks, Aman

vaughnbetz commented 4 years ago

Thanks Aman. Yes, #1 definitely looks like a bug and should be fixed.

2: Sadegh implemented that, but yes, I believe that makes sense and is intentional as you often need a hard block to hit a certain frequency spec, and beyond that you don't think the fabric could keep up. So that naturally leads to a different area-vs-delay trade-off for the hard block (or a search through the space for the smallest block that meets the spec, which can still be facilitated by having a different area-delay trade-off know you can modify).

vaughnbetz commented 4 years ago

@sadegh68 Adding Sadegh in case he has a comment.

aman26kbm commented 4 years ago

Thanks, Dr. Betz. I created a pull request: https://github.com/vaughnbetz/COFFE/pull/33

aman26kbm commented 3 years ago

Closing this issue since a fix has been checked in for #1, and #2 was not something that needed to be fixed.