[libre-riscv-dev] [Bug 316] bperm TODO

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=316

--- Comment #27 from Cole Poirier <colepoirier at gmail.com> ---
(In reply to Luke Kenneth Casson Leighton from comment #25)
>         rb64 = Array([Signal(1, reset_less=True, name=f"rb64_{i}") for i in
> range(64)])
> 
> Cole, this is over the 80-char limit.
> 
> this is why i recommended split it into two: setting up a list,
> followed by assigning the list to an array.
> 
> also, *because* it is assigned to an Array, this code:
> 
>         for i in range(64):
>             m.d.comb += rb64[i].eq(self.rb[i])
> 
> might actually try to generate an Array-assigning method of setting
> the 64 elements.  you'll have to check the graphviz
> 
> the *possibility* of that occurring is why i recommended it to be done as:
> 
>         rb64 = [Signal.... for i in range(64)]
>         for i in range(64):
>             m.d.comb += rb64[i].eq(self.rb[i])
>         rb64 = Array(rb64)
> 
> see the difference?  this one is assigning to signals that, oh look,
> they happen to be in a python list.
> 
> the one that you did, the signals are assigned *via the nmigen Array*.
> 
> and the code is violating PEP8 by being over 80 chars in length.

Thanks for catching this error Luke. I have made the modification you
recommended, and run autopep8 on the file to ensure proper conformance to
formatting rules. I'm still learning how to read yosys dot diagrams, but it
does appear that this modification changes the graph, so it is possible that
array-assigning methods were being generated.

-- 
You are receiving this mail because:
You are on the CC list for the bug.