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
Clarify documentation for alignment parameters to mention that it is log2 #6
Comments
Could you investigate this? I'm finishing various issues in nMigen right now, and will then be busy reviewing PRs. |
Sure, I'll have a look at it. |
The problematic assignment is this one, which gets nested During elaboration, A more accurate repro would be: from nmigen import *
from nmigen.back import rtlil
class Top(Elaboratable):
def __init__(self, width):
self.width = width
def elaborate(self, platform):
m = Module()
s = Signal(self.width)
o = Signal()
fanin = 0
for bit in s:
fanin |= bit
m.d.comb += o.eq(fanin)
return m
if __name__ == "__main__":
top = Top(width=256)
print(rtlil.convert(top)) ... which makes me wonder whether the bug comes from nmigen-soc or nmigen. |
Unfortunately, the bug comes from the way you are using First, the alignment argument is not an amount, but it is the logarithm of amount. That is, if you wanted each element to be aligned to an address that is a multiple of 8, you would specify Second, the alignment argument is not specified in bits, but in words. That is, if you make a CSR bus which is connected to a 8-bit host bus, and you want registers to be densely packed, you should use:
If you make a CSR bus which is connected to a 32-bit host bus, but want the internal CSR accesses to be 8-bit wide to conserve resources, and want registers to start at 32 bit boundaries, you should use:
Does this make sense? |
Oh, and I still consider this issue a bug, it's just a bug in documentation. |
Yes, thanks for the explanation !
The
|
Yes; but docstrings elsewhere do not, and if you got confused, so will other people. If you feel like you can clarify these docs, I would be grateful, otherwise I'll do it later myself. |
Repro:
Output:
The text was updated successfully, but these errors were encountered: