The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.38k stars 485 forks source link

openlane RePlAce diverged at initial iteration #703

Closed vscheyer closed 2 years ago

vscheyer commented 3 years ago

I'm trying to create a layout using openlane, and I ran into this error that says

RePlAce diverged at initial iteration.
Please tune the parameters again
Error: or_replace.tcl, 94 GPL-0304`

I took a look at the RePlAce repo, but I wasn't able to find this error there: https://github.com/The-OpenROAD-Project/RePlAce

The design is quite small as it's just the digital part of an ADC. I tried formatting the config.tcl file appropriately for a small layout based on this wiki: https://github.com/The-OpenROAD-Project/OpenLane/wiki#how-to-add-a-small-design . I've successfully used config.tcl files for a small design before, but perhaps there something I'm missing here.

I also tried changing some parameters like this:

Here's the verilog (written by my teammate Jonah) for reference. It compiles without errors:

module digADC(

input clk, // input clock from crystal
input Vtripped, //ADC control signal, Vout > 0.9 V
output reg intc, // Controls integrator direction, 0 for sampling, 1 for measuring
output reg[6:0] val1, // 7 bit output value
output reg[6:0] val2, // 7 bit output value
output reg source, // Swaps input source
output reg sample); //Pulses to allow capacitor to sample sensor input

reg [7:0] counter;
reg s1, s2;

initial begin //Everything should be 0
  counter <= 8'd0;
  val1 <= 7'd0;
  val2 <= 7'd0;
  source <= 1'b0;
  s1 <= 0;
  s2 <= 0;
  sample <= 0;
end

always @(*) begin
  intc <= counter[7]; //intc is always just 8th bit of the counter

end

always @(posedge Vtripped) begin
  val1 <= source ? counter[6:0] : val1;
  val2 <= source ? val2: counter[6:0];
  counter <= 8'd0;

end

always @(posedge intc) begin
  source <= ~source; //Swap source select before its important (but after we use it for assigning)
  sample <= intc && !s1; // Gets pulse on rising edge of Vtripped
end

always @(posedge clk) begin
 counter <= Vtripped ? 8'd0 : counter + 8'd1;
 s1 <= intc;
 s2 <= sample;
 sample <= (sample && s2) ? 0 : sample;
end
endmodule

And here's the config.tcl file

# User config
set ::env(DESIGN_NAME) digADC

# Change if needed
set ::env(VERILOG_FILES) [glob $::env(DESIGN_DIR)/src/*.v]

# Fill this
set ::env(CLOCK_PERIOD) "32767"
set ::env(CLOCK_PORT) "clk"
set ::env(CLOCK_NET) $::env(CLOCK_PORT)

set ::env(FP_CORE_UTIL) 5
set ::env(PL_TARGET_DENSITY) 0.5

# set ::env(INIT_DENSITY_PENALTY) 1e-6
# set ::env(PL_SKIP_INITIAL_PLACEMENT) 1

set filename $::env(DESIGN_DIR)/$::env(PDK)_$::env(STD_CELL_LIBRARY)_config.tcl
if { [file exists $filename] == 1} {
    source $filename
}

Any advice would be appreciated. Thank you!

maliberty commented 3 years ago

It would be helpful if you could package this as a standalone test case (eg without source $filename that isn't included).

vscheyer commented 3 years ago

Oh sorry about that! Should I upload the openlane design src folder?

maliberty commented 3 years ago

I don't know the best way to package a test case from OpenLane. @donn is there a mechanism like 'make issue' in OpenRoad-flow-scripts that produces a standalone tgz with all relevant files?

vscheyer commented 3 years ago

I'm happy to write a test script or package this however is helpful, just not sure how to do this. Thanks for pointing me to Donn!

maliberty commented 3 years ago

The ideal is a tgz that when unpacked you can just run "openroad bug.tcl" and the problem is reproduced without needing to have anything more than a working openroad in your path.

maliberty commented 3 years ago

The smallest/fastest reproducer is also desirable.

donn commented 2 years ago

There is no official mechanism but I've been contending with this script here. It works mostly OK.

#!/usr/bin/env ruby

# Copyright 2020 Efabless Corporation
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#      http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script creates a reproducible, self-contained package of files to demonstrate
# OpenROAD behavior in a vaccum, suitable for filing issues.
#
# The result is a tarred, xzipped and gzipped tarball for minimal size while
# maintaining GitHub uploadability.

require 'fileutils'

run_path = "./designs/inverter/runs/openlane_test"
or_script = "./scripts/openroad/or_pdn.tcl"
tarball = true

# --
run_name = File.basename(run_path)
or_script_basename = File.basename(or_script)

# Read Envs
env = {}
rx = /\s*set\s*(::env\(\w+\))\s*(.+)/
load_env = File.read(File.join(run_path, "config.tcl")) + "\n" + File.read("/usr/local/pdk/sky130A/libs.tech/openlane/config.tcl")
for line in load_env.split("\n")
    match = rx.match(line)
    if match.nil?
        next
    end
    env["$#{match[1]}"] = match[2]
    env["[info exists #{match[1]}]"] = "1 == 1"
end

# Set Up Destination
destination_folder = "./_build/#{run_name}_#{or_script_basename}_packaged"

system "rm -rf #{destination_folder}"
system "mkdir -p #{destination_folder}"

# Get Full List Of Files
str_rx = /("((?:[^\\"]|\\\\|\\|\\\[|\\\]")*)"([^\s{}\(\)]*))/

files = { or_script => or_script }
file_copy_list = [or_script]

while filename = file_copy_list.shift
    script_str = File.read(filename)
    for k, v in env
        script_str.gsub!(k, v)
    end

    string_matches = script_str.scan(str_rx)
    current_file_list = []
    for m in string_matches
        full_replacable = m[0]
        in_quotes = m[1]
        after_quotes = (m[2] or "")

        if not in_quotes.start_with?("/openLANE_flow") and not in_quotes.start_with?("/usr/local/pdk")
            next
        end

        full_path = in_quotes + after_quotes

        if full_path.start_with?("/openLANE_flow")
            full_path = full_path.sub("/openLANE_flow", ENV["PWD"])
        end

        if full_path.end_with?(".tcl")
            file_copy_list << full_path
        end

        current_file_list << [full_path, full_replacable]

        files[full_path] = full_replacable
    end
end

for path, replacable in files    
    basename = File.basename(path)
    destination = File.join(destination_folder, basename)

    begin   
        if Dir.exist?(path)
            FileUtils.copy_entry(path, destination)
        else
            file_str = File.read(path)

            if basename.end_with?(".tcl")
                puts "Processing #{basename}..."
                for k, v in env
                    file_str.gsub!(k, v)
                end
                for k, v in files
                    puts "Replacing `#{v}` with `#{File.basename(k)}`"
                    file_str.gsub!(v, File.basename(k)) # Replacable is v
                end
            end

            File.open(destination, "w") { |f|
                f << file_str
            }

        end
    rescue
        STDERR.puts "Couldn't copy #{basename}, skipping..."
    end
end

run_cmd = File.join(destination_folder, "run")
File.open(run_cmd, "w") { |f|
    f << "#!/bin/sh\n"
    f << "dir=$(cd -P -- \"$(dirname -- \"$0\")\" && pwd -P)\n";
    f << "cd $dir; docker run --rm -tiv `pwd`:`pwd` -w `pwd` efabless/openlane:current openroad #{File.basename(or_script)}"
}
system "chmod +x '#{run_cmd}'"

if tarball
    system "tar -cv #{destination_folder} | xz | gzip > ./_build/packaged.tar.xz.gz"
end
rovinski commented 2 years ago

@vscheyer it should probably be noted that your Verilog code is buggy.

  1. initial constructs are not synthesizable in ASICs (only FPGAs). This is because it is not possible to control the state of a register when the power is first turned on. You should instead use a reset signal which is asserted when the device turns on, and then de-asserted when the clock inputs are stable. Something like:

    always @(posedge clk) begin
    if (reset) begin
    counter <= '0;
    end else begin
    counter <= counter + 1;
    end
  2. Generally you should use a blocking assignment = for combinational blocks, not non-blocking <=

    always @(*) begin
    intc <= counter[7];
  3. Your design will infer multiple independent clocks. I really don't think that's what you want. Every time you have always @(posedge [signal]), and [signal] is an input, you are creating a new clock. You should try to rewrite your design such that everything is running off the same clock. If you truly need some of the inputs to be asynchronous (they could change at any time), you should add synchronizers to your design (look up the term Clock Domain Crossing).

  4. As an addition to (3), you have a register counter which is being assigned by multiple signals from different clocks (clk and Vtripped). This is a huge no-no and will probably lead to undefined behavior. It definitely will not do what you want, and I doubt OpenROAD could handle that anyways.

vscheyer commented 2 years ago

@donn thanks for your help! I tried using your ruby script to make a tarball to reproduce my issue. I've never done this before so I hope I did it correctly, please let me know if I need to try again. digADC_packaged.tar.xz.gz

vscheyer commented 2 years ago

@rovinski thank you for those pointers! I'm mostly familiar with using verilog on FPGAs since that's what I've done in classes, so that's helpful to know how initial constructs work with ASICs. The blocking vs. non-blocking advice is also helpful. I'll take a look at the clock situation too! The idea was to have multiple externally generated clocks that are synced up in the rest of the system, but perhaps that will complicate things in OpenROAD.

QuantamHD commented 2 years ago

There's not much in that file. It's missing most of the required information like the def, lib and lef files.

vscheyer commented 2 years ago

@QuantamHD oh yeah that makes sense. Perhaps you or @donn could help me clarify something about generating a usable tarball? From what I can tell, the script that Donn shared (above) requires changes on the following lines. I replaced run_path to match my design (digADC, run 03-06_22-55:

run_path = "./designs/digADC/runs/03-06_22-55"
or_script = "./scripts/openroad/or_pdn.tcl"

What does op_pdn.tcl do? Is that specific to the test case and should I change it?

When I run the script from Donn as set up here, I get the following terminal output which appears to be free of errors but I think I'm missing a step:

Processing or_pdn.tcl...
Replacing `./scripts/openroad/or_pdn.tcl` with `or_pdn.tcl`
Replacing `"/openLANE_flow/designs/digADC/runs/03-06_22-55/tmp/merged_unpadded.lef"` with `merged_unpadded.lef`
./_build/03-06_22-55_or_pdn.tcl_packaged/
./_build/03-06_22-55_or_pdn.tcl_packaged/or_pdn.tcl
./_build/03-06_22-55_or_pdn.tcl_packaged/merged_unpadded.lef
./_build/03-06_22-55_or_pdn.tcl_packaged/run

Thank you!

vscheyer commented 2 years ago

Thanks @maliberty for opening an issue about packaging openlane errors!

donn commented 2 years ago

@vscheyer or_script is supposed to point to the OpenROAD script causing the failure, i.e. in your case, ./scripts/openroad/or_replace.tcl.

Additionally, you should replace "/usr/local/pdk/sky130A/libs.tech/openlane/config.tcl" with wherever you installed your PDK.

vscheyer commented 2 years ago

Thanks @donn ! I changed it so that or_script points to ./scripts/openroad/or_replace.tcl, and I made sure that line 40 points to the config.tcl file for my openlane install. I tried running your script again with seemingly more success. I'm getting a few warnings about missing files though, and I'm not sure if that is detrimental or not. Is there something I should do to fix this?

Couldn't copy replace.timing.rpt, skipping...
Couldn't copy replace.min_max.rpt, skipping...
Couldn't copy replace.rpt, skipping...
Couldn't copy replace_wns.rpt, skipping...
Couldn't copy replace_tns.rpt, skipping...

Here's a new tarball version that has more items in it and is maybe more useful!

digADC_packaged_2.xz.gz

donn commented 2 years ago

Yeah those are normal, they're all outputs. The ruby script cant tell whats an input and whats an output, so it just tries to copy everything.

vscheyer commented 2 years ago

Ok great! Thanks for clarifying. If anyone is able to reproduce the issue using that tarball I'd love to see if I can resolve the diverging RePlAce error. I've tried playing around with the parameters a bit more but still no luck.

maliberty commented 2 years ago

you need to include 7-pdn.def

vscheyer commented 2 years ago

@maliberty Do you mean I need to add it to the tarball? Or I need to do something so it shows up in my design folder to make the error go away? Right now I can find a 7-pdn.def file in digADC/runs/03-06_22-55/tmp/floorplan/

maliberty commented 2 years ago

When I unpack digADC_packaged_2.xz.gz all I see are:

% ls base.sdc merged_unpadded.lef run* digADC.synthesis.v or_replace.tcl trimmed.lib

There is no def.

maliberty commented 2 years ago

More precisely _build/03-06_22-55_or_replace.tcl_packaged/base.sdc _build/03-06_22-55_or_replace.tcl_packaged/merged_unpadded.lef _build/03-06_22-55_or_replace.tcl_packaged/digADC.synthesis.v _build/03-06_22-55_or_replace.tcl_packaged/trimmed.lib _build/03-06_22-55_or_replace.tcl_packaged/run _build/03-06_22-55_or_replace.tcl_packaged/or_replace.tcl

there is no digADC/runs/03-06_22-55/tmp/floorplan directory

donn commented 2 years ago

Try this script @vscheyer: https://github.com/The-OpenROAD-Project/OpenLane/blob/develop/or_issue.py

The ruby script was kind of ad-hoc, this is designed to be more general.

vscheyer commented 2 years ago

Oh awesome, thank you @donn ! I just tried that script and it seems like it worked. Here's the result: 03-06_22-55_or_pdn_packaged.tar.gz

maliberty commented 2 years ago

We are getting warmer as I have the def, but this is a replace issue and you've given a script to run pdngen. Fwiw this script doesn't run either:

[INFO ODB-0134] Finished DEF file: ./tmp/floorplan/7-pdn.def can't read "::env(PDN_CFG)": no such variable

@donn would you fix this issue with the script not getting all the dependencies and not setting all the envars, and advise on how to properly package a global placement issue.

donn commented 2 years ago

@vscheyer Ensure that you pass -s ./scripts/openroad/or_replace.tcl instead of -s ./scripts/openroad/or_pdn.tcl

vscheyer commented 2 years ago

Ah my bad! Here's the result using -s ./scripts/openroad/or_replace.tcl 03-06_22-55_or_replace_packaged.tar.gz

maliberty commented 2 years ago

The data is all there but I had to update the script to the current OR style:

read_lef ./tmp/merged_unpadded.lef read_def ./tmp/floorplan/7-pdn.def global_placement -density 0.5

When I run that through the current OR I don't get any error:

INFO GPL-0021] MacroInstsArea: 0 [InitialPlace] Iter: 1 CG Error: 0.00000006 HPWL: 920280 [InitialPlace] Iter: 2 CG Error: 0.00000000 HPWL: 109720 [InitialPlace] Iter: 3 CG Error: 0.00000000 HPWL: 109720 [InitialPlace] Iter: 4 CG Error: 0.00000000 HPWL: 109720 [InitialPlace] Iter: 5 CG Error: 0.00000000 HPWL: 109720 [INFO GPL-0031] FillerInit: NumGCells: 151 [INFO GPL-0032] FillerInit: NumGNets: 19 [INFO GPL-0033] FillerInit: NumGPins: 36 [INFO GPL-0023] TargetDensity: 0.50 [INFO GPL-0024] AveragePlaceInstArea: 3753600 [INFO GPL-0025] IdealBinArea: 7507200 [INFO GPL-0026] IdealBinCnt: 166 [INFO GPL-0027] TotalBinArea: 1252451200 [INFO GPL-0028] BinCnt: 8 8 [INFO GPL-0029] BinSize: 4428 4420 [INFO GPL-0030] NumBins: 64 [NesterovSolve] Iter: 1 overflow: 0 HPWL: 201556 [NesterovSolve] Iter: 10 overflow: 0 HPWL: 201556 [NesterovSolve] Iter: 20 overflow: 0 HPWL: 201556 [NesterovSolve] Iter: 30 overflow: 0 HPWL: 201556 [NesterovSolve] Iter: 40 overflow: 0 HPWL: 201556 [NesterovSolve] Iter: 50 overflow: 0 HPWL: 201556 [NesterovSolve] Finished with Overflow: 0.000000

So this is really an issue with OpenLane using an older version of the tool. There is nothing to fix in the current OR so I'm going to close this.

donn commented 2 years ago

Well, hey! At least we verified the packager script works.

Sorry for the inconvenience, @vscheyer. I'll ping you when OpenLane's version of OR is up to date.

vscheyer commented 2 years ago

Thanks for your help @donn and @maliberty ! I'll keep an eye out for the OR update in OpenLane