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

strange error with arrays #51

Closed
rroohhh opened this issue Mar 26, 2019 · 5 comments
Closed

strange error with arrays #51

rroohhh opened this issue Mar 26, 2019 · 5 comments
Labels
Milestone

Comments

@rroohhh
Copy link
Contributor

rroohhh commented Mar 26, 2019

This

from nmigen import *
from nmigen.cli import main

m = Module()

a = Signal(1)
array = Array(x for x in range(2))

with m.If(0 == array[a]):
    m.d.sync += a.eq(0),        

if __name__ == "__main__":
    main(m, ports=[a])

gives a strange error (https://hastebin.com/cedapovure.sql) when running generate

while

from nmigen import *
from nmigen.cli import main

m = Module()

a = Signal(1)
array = Array(x for x in range(2))

dummy = Signal(1)
m.d.comb += a.eq(dummy)

with m.If(0 == array[a]):
    m.d.sync += dummy.eq(0),
        

if __name__ == "__main__":
    main(m, ports=[a])

works.

I am not sure how to interpret this error and it seems like a bug in nmigen.

@whitequark whitequark added the bug label Mar 26, 2019
@whitequark
Copy link
Contributor

Yup, it's a bug and I think I know where.

@rroohhh
Copy link
Contributor Author

rroohhh commented Mar 26, 2019

I don't know if it helps, but I reduced it a bit further:

from nmigen import *
from nmigen.cli import main

m = Module()

a = Signal(1)
array = Array(x for x in range(2))

m.d.sync += a.eq(array[a])
        
if __name__ == "__main__":
    main(m, ports=[a])

fails similarly (https://hastebin.com/qujiciyihe.sql).

@whitequark
Copy link
Contributor

Oh, I figured it out, thanks! It's an issue with how legalizer expands arrays into switches; it does that by replacing the array index with a constant. But, in this case, you also have the array index on left-hand side, which shouldn't be replaced with a constant.

@rroohhh
Copy link
Contributor Author

rroohhh commented Mar 26, 2019

Ah, I understand. So part also suffers from the same problem:

from nmigen import *
from nmigen.cli import main

m = Module()

p = Signal(5, reset=0b11)
a = Signal(2, reset = 1)

m.d.sync += Cat(p.part(a, 1), a).eq(a)

if __name__ == "__main__":
    main(m)

Actually looking at the Part handling I think I noticed another bug: #52.

@whitequark
Copy link
Contributor

Thanks for the report! By the way, you can use Array(range(2)) just as well, you don't need the x for x in ... part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants