<sb0>
whitequark: x's are a bit messy (in verilog at least) and are one more thing to handle. what about selecting the last element?
Gurty has quit [Ping timeout: 252 seconds]
rohitksingh has quit [Ping timeout: 245 seconds]
rohitksingh has joined #m-labs
Gurty has joined #m-labs
Gurty has quit [Changing host]
Gurty has joined #m-labs
_rht has joined #m-labs
_whitelogger has joined #m-labs
hozer has joined #m-labs
_whitelogger_ has joined #m-labs
_whitelogger has quit [Ping timeout: 250 seconds]
_whitelogger has joined #m-labs
_rht has quit [Quit: Connection closed for inactivity]
_whitelogger has joined #m-labs
<whitequark>
sb0: regarding one more thing to handle, i think this happens either way
<whitequark>
specifically. x are trivial to emit in rtlil. migen simulation should probably not have x. however, migen simulation can just crash if you request an out of bounds array element.
<whitequark>
but even if it doesn't crash, any such behavior has to be coded for and tested explicitly.
<whitequark>
which is why i'm asking.
<whitequark>
right now it crashes but without a nice message
<whitequark>
nmigen simulation*
<whitequark>
the main reason i want x is synthesis quality. right now migen output ranges from slightly bad for synthesis to positively atrocious
<whitequark>
for example migen output completely breaks fsm detection in yosys because the state register has an init value and results in orders of magnitude more logic than necessary
<whitequark>
well, maybe 1 or 1.5 OOM more.
<whitequark>
rjo: btw. nmigen.back.pysim is currently several times slower than migen.sim. do not be discouraged. i think it is accidentally quadratic, or maybe some higher power of n.
<_whitenotifier-f>
[nmigen] jordens commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN2u
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN2z
<_whitenotifier-f>
[m-labs/nmigen] whitequark pushed 3 commits to master [+0/-0/±7] https://git.io/fpN2H
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN2N
<_whitenotifier-f>
[nmigen] jordens commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNV6
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNwB
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNwH
_whitelogger has joined #m-labs
<cr1901_modern>
What is the acronym "NFC"?
<whitequark>
no functional change
<whitequark>
sb0: ping
<_whitenotifier-f>
[m-labs/nmigen] whitequark pushed 1 commit to master [+0/-0/±1] https://git.io/fpNK3
<_whitenotifier-f>
[m-labs/nmigen] whitequark 9794e73 - back.rtlil: reorganize value compiler into LHS/RHS.
<_whitenotifier-f>
[nmigen] jordens commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNKN
rohitksingh has quit [Ping timeout: 246 seconds]
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN6q
<_whitenotifier-f>
[nmigen] jordens commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN64
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpN6r
<sb0>
whitequark: i've only used migen with the xilinx tools and they have no problem detecting the FSMs
<whitequark>
sb0: yes, xilinx doesn't have this problem. i think nothing based on synopsys has.
<sb0>
whitequark: does x improve synthesis quality a lot?
<sb0>
whitequark: and you have to balance it against simulation-synthesis mismatches and more hardware non-determinism
<sb0>
whitequark: the convention that out-of-bounds accesses results in the last element being returned gives completely deterministic behavior
<sb0>
with x, it could potentially even return a different value depending on the magnitude of the overflow
<whitequark>
sb0: re synthesis quality: migen's reset value convention already results in a lot of extra muxes per signal. sometimes these muxes get removed, sometimes not. i would like to avoid making the problem worse.
<sb0>
and that value could even change between synthesis runs with changes in other parts of the code. that can quickly result in debugging hell.
<whitequark>
regarding out-of-bounds access, there are two parts here
<whitequark>
first, migen already does not promise you this. afaict the docs do not guarantee you any particular behavior.
<whitequark>
second, what you are describing is the case where your design is *already* buggy
<sb0>
what it does is maybe not well documented, but it does something deterministic
<sb0>
and also does not use x, which brings additional non-determinism
<sb0>
sure, but when you're in FPGA hell, it makes it easier to reason when things behave in a predictable manner
<whitequark>
so let's separate synthesis and simulation here.
<whitequark>
are you saying that OOB access should be defined in simulation too?
<whitequark>
or are you only talking about determinism in synthesis here?
<sb0>
sure. simulation should match hardware.
<sb0>
and everything should be as deterministic and predictable as possible
<whitequark>
i do not think that OOB access returning last element is predictable.
<sb0>
it is more predictable than x and more predictable than returning a pseudo-random element
<whitequark>
it is less predictable than an exception.
<sb0>
returning 0 is also acceptable, but I suspect this creates more logic than returning the last element
<whitequark>
in simulation, an exception on OOB access is obviously the right choice.
<sb0>
yes, but what if you cannot reproduce the situationthat produces the OOB access in the simulator? while your hardware is crashing?
<sb0>
also, if you ignore the result of the array access, you don't care if the index is out of bounds
<sb0>
so that exception would kick in at inappropriate times
<whitequark>
14:19 < sb0> yes, but what if you cannot reproduce the situationthat produces the OOB access in the simulator? while your hardware is crashing?
<whitequark>
well, an acceptable compromise is exception in simulation and deterministic behavior in hardware by default.
<whitequark>
with a default-off option to use x instead
<whitequark>
btw
<whitequark>
migen already produces x in some cases.
<whitequark>
if you use s.part() to shift out of bounds, x is shifted in.
<sb0>
i guess that slipped past the review of the part() PR
<whitequark>
well, it would have slipped past me. I had to look at Yosys' IR generated for Verilog [x+:y] construct
<whitequark>
and it lowers to a $shiftx cell
<sb0>
and migen doing something wrong is not a valid reason for continuing to do it that way
<whitequark>
I agree with that
<whitequark>
14:20 < sb0> so that exception would kick in at inappropriate times
<whitequark>
I'm not convinced the behavior with exception would result in a net increase of bugs as opposed to net decrease
<sb0>
that's about balancing synthesis results
<whitequark>
balancing?
<sb0>
if you have to ensure that you always index the array correctly, even when you don't care about the result of the array access, you need additional logic that results in worse timing
<whitequark>
but that's exactly the same logic that will be generated if you don't use x
<whitequark>
sb0: ok, I have an idea for a different compromise.
<whitequark>
let's add an argument to Array(), call it "in_bounds".
<whitequark>
for POT Arrays, nothing changes.
<whitequark>
for NPOT Arrays, the argument becomes mandatory in nMigen.
<sb0>
POT?
<whitequark>
if you pass in_bounds=False, then the behavior is deterministic and e.g. chooses the last element. whatever Migen currently happens to do.
<whitequark>
power-of-two
<whitequark>
that happens in both synthesis and simulation.
<whitequark>
if you pass in_bounds=True, then in simulation, exception is thrown, and in synthesis, X is used.
<whitequark>
this is similar to what LLVM uses to handle the same issue
<whitequark>
in practice in_bounds=False will just pad the array
<sb0>
what about a POT array with a large signal that can index more than the array?
<whitequark>
hmm
<whitequark>
ok, rephrasing to take that into account:
<whitequark>
a) if in_bounds=None (not specified), an exact match is required between array depth and 2**index width.
<sb0>
also there aren't so many POT arrays, so I'm not sure that making that distinction will help
<whitequark>
b) if in_bounds=False, the current Migen behavior is preserved.
<whitequark>
c) if in_bounds=True, any OOB access results in exception or x
<whitequark>
well, in my code, I think all Arrays are intended to be POT, actually
<whitequark>
let me grep artiq
<sb0>
x is really causing debugging trouble. since many complex FPGA developments are dominated by debugging, I think it always ought to be avoided
<whitequark>
sb0: so what happens when I want to remove this extra logic generated because I can't specify x?
<whitequark>
do I write out multiplexers by hand?
<sb0>
it's a costly optimization if it results in weeks of FPGA hell. it should at least be disabled by default, and only people who have already debugged their design and want to squeeze the last bit of performance/area can maybe consider using x.
<whitequark>
ok. what about this.
<sb0>
if x results in large timing gains, maybe we can actually apply this principle globally. I've been doing mostly verilog, where there isn't an easy way to disable x, so I have learned to avoid it like the plague - but the situation is different with *migen
<whitequark>
hm. x has an important mechanism in verilog, in that it enables certain kinds of verification.
<whitequark>
think about x this way: it is like UB in C.
<whitequark>
signed overflow is UB, which is bad if your compiler removes half your code after detecting an overflow.
<whitequark>
but, signed overflow is UB, which means that ubsan can warn each time it happens.
<whitequark>
(and in application code, signed overflow is virtually always a bug, often remotely exploitable bug.)
<whitequark>
it is true that most people do not understand x and do not use x that way, and also tooling support for it sucks anyway.
<sb0>
x is actually quite clumsy to use in verilog sim, which is another (more minor) reason I stopped using it. doing it better requires serious thought.
<whitequark>
true.
<whitequark>
the tooling is really bad, which negates most of the potential advantages of x.
<whitequark>
I think companies like ARM have their custom tooling that makes x useful and usable, and also they use very strict discipline which eliminates many cases where x is a footgun.
<whitequark>
but that's not scalable
<sb0>
okay, well if you can think of a useful design for x, and x gets disabled by default (with the same behavior on hw and sim, i.e. last element is returned on OOB) then we can do it like that
<whitequark>
ok, will do that.
lkcl has quit [Ping timeout: 250 seconds]
_rht has joined #m-labs
lkcl has joined #m-labs
<_whitenotifier-f>
[nmigen] jordens commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNyf
rohitksingh has joined #m-labs
<_whitenotifier-f>
[m-labs/nmigen] whitequark pushed 3 commits to master [+0/-0/±3] https://git.io/fpNSa
<_whitenotifier-f>
[m-labs/nmigen] whitequark 2833b36 - back.rtlil: don't emit a slice if all bits are used.
<_whitenotifier-f>
[m-labs/nmigen] whitequark e86104d - back.rtlil: use slicing to match shape when reducing width.
<_whitenotifier-f>
[nmigen] whitequark commented on issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNNC
<_whitenotifier-f>
[nmigen] whitequark closed issue #7: Decide on the specific simulation and synthesis behavior for out-of-bounds Array access - https://git.io/fpNgL