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

Assume/Assert Cell Support #18

Closed
wants to merge 4 commits into from
Closed

Assume/Assert Cell Support #18

wants to merge 4 commits into from

Conversation

cr1901
Copy link
Contributor

@cr1901 cr1901 commented Jan 1, 2019

Scope

This PR adds the beginning of formal verification support to nmigen. With this PR, a user can now write statements of the following form:

m.d.comb += Assert(foo == bar)
m.d.sync += Assert(baz == 0)

with m.If(quux):
    m.d.comb += Assume(dummy == 1)

The first statement in particular will create Asserts/Assumes analogous to yosys's "properties":

assert property (foo == 0)

A yosys property is syntactic sugar for:

always @(*) assert(foo == 0)

The third statement is analogous to a SystemVerilog immediate assert/assume.

The generated RTLIL can then be fed to yosys, which outputs an smtv2 file. Using yosys-smtbmc or symbiyosys, one can now run a basic formal verification flow using nmigen!

Limitations

  • Something analogous to initial assume/assert not yet supported (easy to add initial support, but I don't know what the semantics should be if they are used within an If/Else/Switch context.
  • You can only do immediate asserts (in SystemVerilog, property is for concurrent asserts, and AFAIK yosys does not support these).

@codecov
Copy link

codecov bot commented Jan 1, 2019

Codecov Report

Merging #18 into master will decrease coverage by 0.82%.
The diff coverage is 40.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
- Coverage   81.31%   80.48%   -0.83%     
==========================================
  Files          19       19              
  Lines        2884     2942      +58     
  Branches      625      632       +7     
==========================================
+ Hits         2345     2368      +23     
- Misses        497      530      +33     
- Partials       42       44       +2
Impacted Files Coverage Δ
nmigen/hdl/dsl.py 99.25% <100%> (ø) ⬆️
nmigen/back/pysim.py 95.2% <100%> (+0.01%) ⬆️
nmigen/hdl/ast.py 81.44% <29.72%> (-3.2%) ⬇️
nmigen/hdl/xfrm.py 94.18% <52.63%> (-3.09%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea7e19e...220c313. Read the comment docs.

raise NotImplementedError("Asserts not yet implemented for Simulator backend.") # :nocov:

def on_Assume(self, stmt):
raise NotImplementedError("Assumes not yet implemented for Simulator backend.") # :nocov:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assumes should be ignored during simulation.


def on_Assume(self, stmt):
# Preserve _en, as the compiler state thinks _en is driven.
return Assume(self.on_value(stmt.test), _en=stmt_en)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo.

@@ -164,6 +176,14 @@ def on_value(self, value):
def on_Assign(self, stmt):
return Assign(self.on_value(stmt.lhs), self.on_value(stmt.rhs))

def on_Assert(self, stmt):
# Preserve _en, as the compiler state thinks _en is driven.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is incorrect; _en is in fact driven. Besides, preserving parts of statements during transforms doesn't really need additional comments.

@@ -328,6 +354,12 @@ def groups(self):
def on_Assign(self, stmt):
self.unify(*stmt._lhs_signals())

def on_Assert(self, stmt):
self.unify(*stmt._lhs_signals())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably just do on_Assert = on_Assign.

@@ -352,6 +384,16 @@ def on_Assign(self, stmt):
if any_lhs_signal in self.signals:
return stmt

def on_Assert(self, stmt):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@cr1901 cr1901 force-pushed the formal branch 4 times, most recently from 3bdf7b8 to 1dfa82d Compare January 2, 2019 00:20
self.test = Value.wrap(test)
if _en is None:
self._en = Signal(1, reset_less=True,
name="assume${}$en".format(src(self.test.src_loc)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This produces utterly gigantic IL statements like connect \A \assert$/home/whitequark/Projects/Boneless-CPU/boneless/gateware/formal.py:25$check. I think it's sufficient to call them something like $assume$en`

@whitequark
Copy link
Contributor

Merged.

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

2 participants