-
Notifications
You must be signed in to change notification settings - Fork 58
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
Expose primitive AND, OR, NAND, NOR, XOR, XNOR operations under descriptive names #147
Comments
We probably want to keep |
Do we need those functions? It's easy enough to do it already with the Python |
@sbourdeauducq You can't do xnor that way, it's somewhat annoying to write, and it results in significantly less readable Verilog output / more bloated RTLIL, with one cell per bit. |
Bikeshed suggestion by @emilazy: |
What's wrong with |
It's much easier to figure out what the semantics of |
Yes. The operation "XNOR of every bit" and the operation "compute even parity" produce the same result but are different to the reader of the code because they show different intent. Also, Also, having to add two imports ( |
IMO the negated versions aren't that helpful anyway. For me |
Or |
I don't know where you got the idea that they will be explained in terms of AND and OR. They will not be. The only reason I'm relating them to AND and OR is because that's what will go to the Verilog output, and Verilog readability is a goal (#98). The actual documentation will be something along the lines of
No, it would be at least |
I would vote for having |
|
Yeah, that's fine with me too. We can always add methods the longer names later, if it turns out they're especially useful. |
Well. You did define and explain them in terms of AND/OR in this issue in the title and in your comments. That's where I got the idea from. ;) Is there really need for the parity thing at all? Is this frequently used outside error detection? (I had thought that the unqualified "parity" is generally understood to mean "odd parity", but that's apparently not the case.) |
It's useful quite occasionally, but it's (a) cheap to add (the method |
I think the name should be likewise for maybe it's worth leaving them as an alternative could be |
|
@programmerjake I agree with your argument, any method with "parity" in it seems bound to be inherently ambiguous. As much as I dislike it, I think adding just |
As an example of what I mean by inherently ambiguous, @emilazy suggests calling it |
Verilog has these and they can be quite useful in bit-twiddling code, potentially reducing bugs as doing the same thing explicitly has a potential to introduce them. (I'm pretty sure I did introduce bugs like that.) Also, we already have
Value.bool
, which is really a reduce-or.Bikeshed: how should they be called? I thinkValue.reduce_or
, etc are OK. But another option isValue.any_bits
for reduce-or,Value.all_bits
for reduce-and,Value.even_bits
for reduce-xor, andValue.odd_bits
for reduce-xnor.Bikeshed: should we even keepValue.bool
? Does everyone agree that "any bit set" is the appropriate semantics for converting any value to a 1-bit one? Migen didn't have it so it's not even a backwards compatibility hazard.The text was updated successfully, but these errors were encountered: