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

GHDL assertions end up in json, nextpnr complains #2068

Closed
antonblanchard opened this issue May 18, 2020 · 17 comments
Closed

GHDL assertions end up in json, nextpnr complains #2068

antonblanchard opened this issue May 18, 2020 · 17 comments

Comments

@antonblanchard
Copy link
Contributor

I'm hitting an issue building microwatt with ghdl/yosys/nextpnr where nextpnr complains about an assertion. I'm not sure if yosys shouldn't emit them, if nextpnr should ignore them, or if they are unsupported and ghdl shouldn't emit them.

A simple example:

module test(zot);
input zot;
always @* assert(zot);
endmodule
yosys -p "read_verilog -sv test.v; synth_ecp5 -json test.json"
nextpnr-ecp5 --um5g-85k --freq 12 --json test.json
ERROR: cell type '$assert' is unsupported (instantiated as 'zot_$assert_A')
@whitequark
Copy link
Member

Duplicate of #177.

@antonblanchard
Copy link
Contributor Author

Thanks, I guess the SystemVerilog reproducer was a bit of a red herring. Should GHDL not emit Assert entries at all, or have an option to not emit them? It seems a bit strange to be doing it there, vs yosys or nextpnr.

@daveshah1
Copy link

It wouldn't be too problematic to drop these cells in the nextpnr frontend, if that is the consensus then I'm happy to do that. But I don't know what would happen with synth_xilinx and the EDIF backend, or xilinx/ice40 going into VPR or arachne-pnr via BLIF.

@whitequark
Copy link
Member

Should we perhaps just delete formal cells at the end of every device-specific flow? I'm not sure what the downsides to that are.

@daveshah1
Copy link

Hypothetically I suppose some kind of post-synthesis verification might need them?

@antonblanchard antonblanchard changed the title SystemVerilog assertions end up in json, nextpnr complains GHDL assertions end up in json, nextpnr complains May 19, 2020
@mikey
Copy link

mikey commented Jun 12, 2020

Is there any movement on this? I retested with the latest docker images in ghdl/synth:nextpnr-ecp5 and we are still hitting this.
FWIW antonblanchard/microwatt#172 is blocked on this.

@whitequark
Copy link
Member

This was rejected in #177.

@antonblanchard
Copy link
Contributor Author

@whitequark I'm a bit confused, these are runtime asserts in the VHDL, and GHDL is passing them through to yosys. Should we get GHDL to strip them all out?

@whitequark
Copy link
Member

The Verilog backend also passes the asserts through to Yosys. The solution offered in #177 was to use the Verilog preprocessor to strip asserts for synthesis. Does VHDL have one?

@mikey
Copy link

mikey commented Jun 15, 2020

@tgingold @eine is this something we could do in ghdl-synth?

@tgingold
Copy link
Contributor

  1. You can use the chformal -remove yosys command to remove all the formal cells. That's an immediate workaround.
  2. We could add a -no-assert or -no-formal options to the ghdl front-end to disable all the assertions.

@mikey
Copy link

mikey commented Jun 15, 2020

@tgingold IMHO a -no-assert would be nicer for microwatt as then we wouldn't need to change our code specifically for ghdl-synth/yosys. We don't use formal at all currently.

@whitequark
Copy link
Member

Wait, you don't use formal? Why do you get assertions in the input netlist then?

@mikey
Copy link

mikey commented Jun 15, 2020

@whitequark we just use asserts. An example that's causing us problems is here.
Happy to take pointers if we are doing this the wrong way.

@tgingold
Copy link
Contributor

@whitequark The possibility to use assertions in VHDL exists since the first standard (1987) and is not exactly considered as formal methods.

It is now possible to use the --no-formal option for synthesis with ghdl.

mikey added a commit to mikey/microwatt that referenced this issue Jun 22, 2020
Add --no-formal as suggested by @tgingold here:
YosysHQ/yosys#2068 (comment)
Which fixes synthesis for yosys/nextpnr

Reduce speed to 40MHz for now so we build.

nextpnr will leave a config file around even when it errors out, so build
to a tmp file and move it when we succeed so we don't confuse make.

Signed-off-by: Michael Neuling <mikey@neuling.org>
@mikey
Copy link

mikey commented Jun 22, 2020

@tgingold this works with microwatt. Thanks.

mikey added a commit to mikey/microwatt that referenced this issue Jun 23, 2020
Add --no-formal as suggested by @tgingold here:
YosysHQ/yosys#2068 (comment)
Which fixes synthesis for yosys/nextpnr

Reduce speed to 40MHz for now so we build.

nextpnr will leave a config file around even when it errors out, so build
to a tmp file and move it when we succeed so we don't confuse make.

Signed-off-by: Michael Neuling <mikey@neuling.org>
mikey added a commit to mikey/microwatt that referenced this issue Jun 23, 2020
Add --no-formal as suggested by @tgingold here:
YosysHQ/yosys#2068 (comment)
Which fixes synthesis for yosys/nextpnr

Reduce speed to 40MHz for now so we build.

nextpnr will leave a config file around even when it errors out, so build
to a tmp file and move it when we succeed so we don't confuse make.

Signed-off-by: Michael Neuling <mikey@neuling.org>
mikey added a commit to mikey/microwatt that referenced this issue Jun 24, 2020
Add --no-formal as suggested by @tgingold here:
YosysHQ/yosys#2068 (comment)
Which fixes synthesis for yosys/nextpnr

nextpnr will leave a config file around even when it errors out, so build
to a tmp file and move it when we succeed so we don't confuse make.

Signed-off-by: Michael Neuling <mikey@neuling.org>
mikey added a commit to mikey/microwatt that referenced this issue Jun 30, 2020
Add --no-formal as suggested by @tgingold here:
YosysHQ/yosys#2068 (comment)
Which fixes synthesis for yosys/nextpnr

nextpnr will leave a config file around even when it errors out, so build
to a tmp file and move it when we succeed so we don't confuse make.

Signed-off-by: Michael Neuling <mikey@neuling.org>
mikey added a commit to mikey/microwatt that referenced this issue Jul 2, 2020
Add --no-formal so that asserts are removed by yosys as nextpnr
doesn't like them.

This was suggested by @tgingold here:
  YosysHQ/yosys#2068 (comment)

Signed-off-by: Michael Neuling <mikey@neuling.org>
@mcejp
Copy link

mcejp commented May 20, 2022

FWIW, the equivalent flag for read_verilog is -noassert

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

No branches or pull requests

6 participants