Make Verification Statements (assert,assume) accept printable

[girishpai]

Fix #2350

Contributor Checklist

• [ ] Did you add Scaladoc to every public function/method?
• [x] Did you add at least one test demonstrating the PR?
• [x] Did you delete any extraneous printlns/debugging code?
• [x] Did you specify the type of improvement?
• [ ] Did you add appropriate documentation in docs/src?
• [x] Did you state the API impact?
• [x] Did you specify the code generation impact?
• [x] Did you request a desired merge strategy?
• [ ] Did you add text to be included in the Release Notes for this change?

Type of Improvement

• new feature / API

API Impact

Current API with strings should still work for Assert/Assume. The base case is made to work with printable (Like in Printf) and the current API is built on top of this. Call Printable.pack(String,data) before calling the new API for this case.

Backend Code Generation Impact

None

Desired Merge Strategy

• Squash

Release Notes

Reviewer Checklist (only modified by reviewer)

• [ ] Did you add the appropriate labels?
• [ ] Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
• [ ] Did you review?
• [ ] Did you check whether all relevant Contributor checkboxes have been checked?
• [ ] Did you mark as Please Merge?

[girishpai]

@jackkoenig please note - have not really added many unit tests - except two of them within the current AssertSpec. Can add more unit tests if you feel there is a need.

[mwachs5]

Should we add something to the mdocs about this? just a note "you can use these in your assert(...) and assume(...) statements too! https://www.chisel-lang.org/chisel3/docs/explanations/printing.html I suppose it would have been better if there was an actual doc about asserts and assumes though so maybe we should save that for a followup

[girishpai]

Should we add something to the mdocs about this? just a note "you can use these in your assert(...) and assume(...) statements too! https://www.chisel-lang.org/chisel3/docs/explanations/printing.html I suppose it would have been better if there was an actual doc about asserts and assumes though so maybe we should save that for a followup

Sure @mwachs5 ! Do I need to know anything specific about updating mdocs? Never done that (or maybe I have forgotten?)

[mwachs5]

You can check out the documentation docs here: https://github.com/chipsalliance/chisel3/tree/master/docs#mdoc

The source for example the file I linked above is here: https://github.com/chipsalliance/chisel3/blob/master/docs/src/explanations/printing.md

Note that particular one doesnt use much mdoc functionality but some of the other ones actually compile-check, and some even emit verilog / error messages

[girishpai]

You can check out the documentation docs here: https://github.com/chipsalliance/chisel3/tree/master/docs#mdoc

The source for example the file I linked above is here: https://github.com/chipsalliance/chisel3/blob/master/docs/src/explanations/printing.md

Note that particular one doesnt use much mdoc functionality but some of the other ones actually compile-check, and some even emit verilog / error messages

Cool - but since as you mentioned we do not have anything about asserts / assumes to begin with - let us do that as a follow up!

[sequencer]

See https://github.com/chipsalliance/chisel3/pull/1968#issuecomment-865336046, maybe this needs some implementation decision from @debs-sifive

[girishpai]

See #1968 (comment), maybe this needs some implementation decision from @debs-sifive

I will let @jackkoenig and @debs-sifive chime in here. As this PR indicates - adding Printable support is reasonably easy and straightforward - not sure how much of an effort the suggestion within https://github.com/chipsalliance/chisel3/pull/1968#issuecomment-865336046 is.

[ekiwi]

I will let @jackkoenig and @debs-sifive chime in here. As this PR indicates - adding Printable support is reasonably easy and straightforward - not sure how much of an effort the suggestion within #1968 (comment) is.

From what I understand this PR just adds support for p"{wire}" style printing in addition to "%d", wire and that is awesome. The thing that @debs-sifive mentioned can be added in the background. It would just involve adding an annotation from chisel that points to both, the printf and the verification statement generated. This is orthogonal to this PR.

[girishpai]

I will let @jackkoenig and @debs-sifive chime in here. As this PR indicates - adding Printable support is reasonably easy and straightforward - not sure how much of an effort the suggestion within #1968 (comment) is.

From what I understand this PR just adds support for p"{wire}" style printing in addition to "%d", wire and that is awesome. The thing that @debs-sifive mentioned can be added in the background. It would just involve adding an annotation from chisel that points to both, the printf and the verification statement generated. This is orthogonal to this PR.

Just to add since this accepts any printable - once this other enhancement PR goes in https://github.com/chipsalliance/chisel3/pull/2528 will be able to do cf"Chisel Bits Foo = $foo%x Scala Float = $scalafloatVal%2.2f" as well!

[girishpai]

@jackkoenig incorporated most of your feedback - except the one to make these new macro definitions private - compiler complained and hence have just added a comment on top of each of them - explicitly asking not to use directly.

[jackkoenig]

Replaced by https://github.com/chipsalliance/chisel3/pull/2663