Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

abc9: tolerate &mfs failure by writing output file before calling it (and using that if it fails) #1974

Merged
merged 2 commits into from
Apr 23, 2020

Conversation

eddiehung
Copy link
Collaborator

@eddiehung eddiehung commented Apr 20, 2020

Since &mfs can give a decent QoR improvement, I'm loathe to flat out disable it (my first approach). This PR does two things:
(a) write the output file before every &mfs call
(b) tolerate ABC9 with nonzero exit code (downgrading error to a warning) if output file exists

The idea is that even if &mfs fails -- at least with the default script -- we can still use the pre-&mfs result as if it was disabled. This approach makes less sense when using some more complex scripts like:

RTLIL::constpad["abc9.script.flow"] = "+&scorr; &sweep;" \
"&dch -C 500;" \
/* Round 1 */ \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
/* Round 2 */ \
"&st; &sopb;" \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
/* Round 3 */ \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;";
// Based on ABC's &flow2
RTLIL::constpad["abc9.script.flow2"] = "+&scorr; &sweep;" \
/* Comm1 */ "&synch2 -K 6 -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
/* Comm2 */ "&dch -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
"&load; &st; &sopb -R 10 -C 4; " \
/* Comm3 */ "&synch2 -K 6 -C 500; &if -m "/*"-E 5"*/" {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
/* Comm2 */ "&dch -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save; "\
"&load";

in which multiple mapping rounds using &save/&load may be superior to one round with &mfs.

In those cases, you may wish to disable &mfs explicitly using:
scratchpad -set abc9.nomfs 1
before:
synth_{ecp5,xilinx} -abc9 (the two architectures known to trigger it so far)

Can sometimes fire an assertion, e.g. #1962
@daveshah1
Copy link

Horrible suggestion: given &mfs only rarely breaks and does give a QoR improvement, what about detecting the failure and only then rerunning abc without &mfs in the script?

@eddiehung
Copy link
Collaborator Author

eddiehung commented Apr 20, 2020

Horrible suggestion: given &mfs only rarely breaks and does give a QoR improvement, what about detecting the failure and only then rerunning abc without &mfs in the script?

Less horrible: write out the result before and after &mfs, and prefer the latter.

EDIT: This would only make sense if using an ABC9 script that calls &mfs at the end, which isn't the case these:

RTLIL::constpad["abc9.script.flow"] = "+&scorr; &sweep;" \
"&dch -C 500;" \
/* Round 1 */ \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
/* Round 2 */ \
"&st; &sopb;" \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
/* Round 3 */ \
/* Map 1 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &dsdb;" \
/* Map 2 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;" \
"&st; &syn2 -m -R 10; &dsdb;" \
"&blut -a -K 6;" \
/* Map 3 */ "&unmap; &if {C} {W} {D} {R} -v; &save; &load; &mfs;";
// Based on ABC's &flow2
RTLIL::constpad["abc9.script.flow2"] = "+&scorr; &sweep;" \
/* Comm1 */ "&synch2 -K 6 -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
/* Comm2 */ "&dch -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
"&load; &st; &sopb -R 10 -C 4; " \
/* Comm3 */ "&synch2 -K 6 -C 500; &if -m "/*"-E 5"*/" {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save;"\
/* Comm2 */ "&dch -C 500; &if -m {C} {W} {D} {R} -v; &mfs "/*"-W 4 -M 500 -C 7000"*/"; &save; "\
"&load";

Re-enable mfs for xilinx/ecp5 speculatively -- if it fails, use pre-mfs
result
@eddiehung
Copy link
Collaborator Author

eddiehung commented Apr 20, 2020

@daveshah1 What do you think of 3d7b983? I've left the abc9.nomfs option behind as using other flows without &mfs could mean it may still be superior to the result just before the first &mfs (plus there are &save/&load considerations too).

I've tested it by forcing an assert inside my out-of-tree ABC build as I don't have a sufficiently small testcase...

@daveshah1
Copy link

Seems reasonable to me, I don't have a testcase to hand either though.

@eddiehung eddiehung changed the title xilinx/ecp5: disable abc9's "&mfs" optimisation abc9: tolerate ABC9 with nonzero exit code if output file exists; write output before every &mfs call Apr 20, 2020
@eddiehung eddiehung changed the title abc9: tolerate ABC9 with nonzero exit code if output file exists; write output before every &mfs call abc9: tolerate &mfs failure by writing output file before calling it Apr 20, 2020
@eddiehung eddiehung changed the title abc9: tolerate &mfs failure by writing output file before calling it abc9: tolerate &mfs failure by writing output file before calling it (and using that if it fails) Apr 20, 2020
@kost
Copy link

kost commented Apr 21, 2020

Just tested and can confirm it works.

I have used 215ac18f736cff06a4186e11b9abaa3d669f32e7 commit hash of dev branch of https://github.com/SpinalHDL/SaxonSoc.git which is known to cause this issue.

@eddiehung
Copy link
Collaborator Author

Thanks @kost! Would you mind uploading the Verilog somewhere, I can't seem to get it working. Even better would be if you could minimise the design to find a small testcase that triggers it -- in my experience that can be quite hit-or-miss so don't worry if you're not successful.

@kost
Copy link

kost commented Apr 22, 2020

@eddiehung managed to extract the generated verilog and it is located here:
http://misc.linux.hr/ulx3s/yosys-bugs/yosys-abc9-sigabrt.tar.gz

Just untar it and run make. Just tested that it generates the error reported.

Minimisation of the Verilog I would leave to experts like you. If you're able to document how you do it, i'll be happy to do it myself next time.

@eddiehung
Copy link
Collaborator Author

@eddiehung managed to extract the generated verilog and it is located here:
http://misc.linux.hr/ulx3s/yosys-bugs/yosys-abc9-sigabrt.tar.gz

Just untar it and run make. Just tested that it generates the error reported.

Minimisation of the Verilog I would leave to experts like you. If you're able to document how you do it, i'll be happy to do it myself next time.

Thanks @kost, really appreciate you doing that. I can reproduce the assertion fail on latest master (db27f2f). I'm in the process of minimising it right now, once it succeeds I'll explain my process (which I've made much less ad-hoc now that I have to explain it to someone!)

However, even after all that, the problem I've run into the past and am running into here is that any testcase for this is very brittle. The same case doesn't fire an assertion on 57d6bd3 of #1926 which applies ABC9 in a slightly different way.

@eddiehung
Copy link
Collaborator Author

Okay, here's my method, which uses bugpoint added in #783. The general idea is that it iteratively starts removing modules/ports/cells/etc. in an effort to minimise the design into a smaller testcase.

Use:

../yosys -p "synth_ecp5 -abc9 -run :map_luts; bugpoint -yosys ../yosys -script bugpoint.ys; write_verilog -norename testcase.v" Ulx3sLinuxUboot.v pll_linux.v hdmi.v

Where bugpoint.ys contains:

read_verilog -lib -specify +/ecp5/cells_sim.v +/ecp5/cells_bb.v
synth_ecp5 -abc9 -run map_luts:map_cells

The first command runs synth_ecp5 up until the map_luts label which invokes ABC9. Then the bugpoint pass takes over by removing part of the design and then running bugpoint.ys in a new Yosys process containing the candidate design, and where bugpoint.ys will load the blackboxes again and run just the map_luts label.

This gives an output like this:

<<SNIP>>
ERROR: ABC: execution of command "/home/eddie/yosys/yosys-abc -s -f /tmp/yosys-abc-Og9A3d/abc.script 2>&1" failed: return code 134.
Trying to remove module \Ulx3sLinuxUboot.
Testcase does not crash.
Trying to remove module port \system_usbKeyboardA_usb_dp.
ERROR: ABC: execution of command "/home/eddie/yosys/yosys-abc -s -f /tmp/yosys-abc-UL0zSR/abc.script 2>&1" failed: return code 134.
Testcase crashes.
Trying to remove module port \system_usbKeyboardA_usb_dn.
ERROR: ABC: execution of command "/home/eddie/yosys/yosys-abc -s -f /tmp/yosys-abc-JKkJYK/abc.script 2>&1" failed: return code 134.
Testcase crashes.
Trying to remove module port \system_usbKeyboardA_usbPu_dp.
ERROR: ABC: execution of command "/home/eddie/yosys/yosys-abc -s -f /tmp/yosys-abc-g8FPlT/abc.script 2>&1" failed: return code 134.
Testcase crashes.
<<SNIP>>

The current setup is naive and slow, and despite running it overnight it didn't reach a fixed point so I gave up. But if I was to refine the script, parallelise bugpoint, then we could expect a minimised testcase.

@eddiehung eddiehung merged commit b048afc into master Apr 23, 2020
@eddiehung eddiehung deleted the eddie/abc9_disable_mfs branch April 23, 2020 13:43
@pointcheck
Copy link

Guys, is &mfs in ABC still broken ? I cannot compile my two year old project based on VexRiscV (pretty basic one), it asserts at this very same please with -abc9 option enabled . If I disable -abc9, it passes synth ok, but the design does not fit into ECP5 (25k). Two years ago it worked fine, -abc9 worked and it did a load of optimization producing working bitstream. Please advise.

@CambridgeComp
Copy link

Hi, it seems to be still broken :

68.21.5. Executing ABC9.
Running ABC command: "/yosys-abc" -s -f /abc.script 2>&1
ABC: ABC command line: "source /abc.script".
ABC:
ABC: + read_lut /input.lut
ABC: + read_box /input.box
ABC: + &read /input.xaig
ABC: + &ps
ABC: /input : i/o = 835/ 630 and = 7481 lev = 46 (5.91) mem = 0.14 MB box = 429 bb = 380
ABC: + &scorr
ABC: Warning: The network is combinational.
ABC: + &sweep
ABC: + &dc2
ABC: + &dch -f
ABC: + &ps
ABC: /input : i/o = 835/ 630 and = 9442 lev = 27 (3.92) mem = 0.16 MB ch = 1401 box = 429 bb = 380
ABC: + &if -W 300 -v
ABC: K = 7. Memory (bytes): Truth = 0. Cut = 60. Obj = 140. Set = 636. CutMin = no
ABC: Node = 9442. Ch = 1088. Total mem = 1.90 MB. Peak cut mem = 0.30 MB.
ABC: P: Del = 5157.00. Ar = 10144.0. Edge = 12494. Cut = 102955. T = 0.02 sec
ABC: P: Del = 4991.00. Ar = 9903.0. Edge = 12237. Cut = 101767. T = 0.03 sec
ABC: P: Del = 4991.00. Ar = 4012.0. Edge = 9319. Cut = 242423. T = 0.05 sec
ABC: F: Del = 4990.00. Ar = 2755.0. Edge = 8181. Cut = 202456. T = 0.05 sec
ABC: A: Del = 4987.00. Ar = 2576.0. Edge = 7525. Cut = 191173. T = 0.07 sec
ABC: A: Del = 4987.00. Ar = 2557.0. Edge = 7495. Cut = 197294. T = 0.07 sec
ABC: Total time = 0.29 sec
ABC: + &write -n /output.aig
ABC: + &mfs
ABC: Assertion failed: (iLitNew >= 0), function Gia_ManInsertMfs, file src/aig/gia/giaMfs.c, line 386.
Warning: ABC: execution of command ""/Users/me/Applications/oss-cad-suite/libexec/yosys-abc" -s -f /var/folders/f3/3fbt1wk95112sbbw8wbk8t640000gn/T/yosys-abc-tweXaM/abc.script 2>&1" failed: return code 134.

@pointcheck
Copy link

Will try to report it upstream.

@CambridgeComp
Copy link

//=========================================================================
// Baud rate generators
//=========================================================================

reg        r_tx_ena;
reg [11:0] v_tx_gen; // no error when moved here !!

always@(posedge bus_rst or posedge bus_clk) begin : TX_BAUD_RATE

//reg [11:0] v_tx_gen; // if declared here .... error with return code 134 !!!!!! //

    if (bus_rst) begin
        r_tx_ena <= 1'b0;
        v_tx_gen <= 12'd0;
    end
    else begin
        // Transmitter
        if (v_tx_gen == 12'd0) begin

            v_tx_gen <= 12'd216; // 115200 @ 25 MHz
            //v_tx_gen <= 12'd433; // 115200 @ 50 MHz
            //v_tx_gen <= 12'd867; //115200 @ 148.5 MHz
            //v_tx_gen <= 12'd1288; //115200 @ 148.5 MHz
            //v_tx_gen <= 12'd1301; //115200 @ 150 MHz
            //v_tx_cnt <= 12'd1170; //115200 @ 135 MHz
            //v_tx_cnt <= 12'd867; // 115200 @ 100 MHz
            //v_tx_cnt <= 12'd703; // 115200 @ 81 MHz
            //v_tx_cnt <= 12'd86;  // 115200 @ 10 MHz
            //v_tx_cnt <= 12'd171; // 115200 @ 20 MHz

            r_tx_ena <= 1'b1;
        end
        else begin
            v_tx_gen <= v_tx_gen - 12'd1;
            r_tx_ena <= 1'b0;
        end
    end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants