Verilog Operator Docs #314
Conversation
Pull Request Test Coverage Report for Build 1214
💛 - Coveralls |
1 similar comment
Pull Request Test Coverage Report for Build 1214
💛 - Coveralls |
|
This looks good. I haven't had a chance to review all the TODOs. But here are some comments.
|
There was a problem hiding this comment.
Looks good overall. Agree with Pat that type conversions would be good to include (somewhere, not nec. here).
Perhaps it would be good to have a note somewhere that we can use functions instead of operators wherever python's syntax doesn't allow overloading. And that is in fact a benefit of embedding in python. Just to verilog users understand that lack of certain operators is not a fundamental limitation.
docs/operators.md
Outdated
| | `=` | `m.wire`, **TODO (=)** | Any | All | Assignment cannot be overloaded for arbitrary Python variables, so in general we must use `m.wire`. There are plans to add support for assignment to attributes of magma types, such as `reg.I = io.I`. | | ||
| | `+=`, `-=`, `/=`, `*=` | `None` | None | All | Again, unsupported due to the lack of support for overloading assignment. May be added in the future for attributes of magma types | | ||
| | `%=` | `None` | None | All | See above | | ||
| | `&=`, `|=`, `^=` | `None` | None | All | See above | |
There was a problem hiding this comment.
nit: formatting messed up here because of | being used as a table separator.
docs/operators.md
Outdated
| | Verilog Operator | Magma Operator | Types | Context | Comments | | ||
| |------------------|----------------| ----- | ------- | -------- | | ||
| | `!` | **TODO** | `m.Bit`, `m.Bits` | All | Logical operators like `not` cannot be overloaded in Python. Planned support for a mantle function `m.lnot` as an alternative | | ||
| | `~`, `&`, `~&`, `|`, `~|`, `^`, `~^`, `^~` | **TODO** | `m.Bits` | All | Python does not have built-in support for reduction operators. Use the python `reduce` function instead, e.g. `reduce(mantle.nand, value)`. | |
There was a problem hiding this comment.
nit: formatting, same as above.
docs/operators.md
Outdated
| | Verilog Operator | Magma Operator | Types | Context | Comments | | ||
| |------------------|----------------| ----- | ------- | -------- | | ||
| | `<<`, `>>` | `<<`, `>>` | `m.Bits` | All | **TODO: What does verilog expect for bit width of the shift value? What does magma expect?** | | ||
| | `&&`, `||` | **TODO** | `m.Bits` | All | Python doesn't support overloading logical and and or, we can provide mantle functions instead | |
There was a problem hiding this comment.
nit: formatting, same as above
These changes add a table to
docs/operators.mdwhich provides a reference for Verilog user. It lists all the operators detailed in the IEEE SystemVerilog specification and shows their verilog counterpart.Naturally, magma's operators are not feature complete, so I've attempted to note with TODOs where we could add missing operators (or corresponding mantle functions for Python operators that we can't overload), and other operators which I don't think we should plan to support (e.g.
distfor constraints which is a verification feature).This is mainly a documentation change, but it would be good to get a review on the TODO items and discuss what our plans for that are. If we decide to add them, we can open a separate github issue to track and discuss design.