<agg>
modwizcode: sorry, I had to run, but I was thinking about your thing, maybe this is simpler? https://paste.debian.net/1188616/
<agg>
on each clock cycle it outputs data consisting of some bytes from the previous clock cycle and some from the clock cycle before that
<agg>
if you changed the final sync to comb, it would instead output bytes from this clock cycle and the one previous
<modwizcode>
so still 3 cycle then right (I'm looking it over now)
<modwizcode>
unless you change the final sync to comb
<agg>
that's the best you can hope for - with registered outputs the output will never contain this cycle's input data
<agg>
i'd consider it two or even one cycle though, rather than three
<modwizcode>
right
<agg>
(if you change the final sync to comb it still works fine, the output is still correct and is one cycle quicker, it's just you might find timing becomes harder)
<agg>
i'd count it from the output containing the previous cycle's data, so the latency through it is only one cycle
<agg>
(but it does need two cycles to fill the pipeline, since the output can contain data from two cycles ago)
<modwizcode>
Is that how most people count latency? Idk why I decided that it's 3 probably because it's valid on 3 when buffered, but like that's only 2 cycles of delay
<agg>
it doesn't need three cycles though
<agg>
I think given as data from cycle n appears on the output at cycle n+1 it's fair to call it one cycle of latency
<modwizcode>
yeah
<agg>
bit_select works on signals btw, saving you that switch and gluing together two halves
<modwizcode>
Yeah there was a reason I was doing it like that
<modwizcode>
the initial reason was because initially I wrote this as a direct conversion of someone else's verilog
<agg>
the stack of if statements in mine is similar but not quite identical to the Encoder you use
<modwizcode>
this copy I posted was a rewrite from memory, I just couldn't think of how to express the logic until after I wrote it :)
<modwizcode>
I used the encoder because it seemed more reuse friendly
<agg>
it won't set align when no input bits are set, but it will set when multiple are set
<modwizcode>
what's funny is that this was my exercise in using nmigen for real
<modwizcode>
so the first pass of this I wrote it out as in the verilog
<modwizcode>
and then I realized I could just use a for loop, and then it looked way too common an op and then I realized *duh* that's an encoder lmao
<modwizcode>
unfortunately the encoder boilerplate is basically equal to just writing the logic inline :/
<agg>
yea, not really much benefit here I don't think
<agg>
except that the behaviour is subtly different wrt detecting invalid states
<agg>
I kinda find the for loop version a bit clearer to read, especially since "Encoder" isn't obviously one-hot-to-binary by name
<modwizcode>
I think in hardware it should mostly shake out?
<modwizcode>
Yeahhhh
<agg>
but also this way you can add your actual condition very easily
<modwizcode>
In my initial pass (who knows where that copy is) I had a comment like "this says encoder but it's more like a decoder from one-hot to index"
<agg>
with m.If(self.rx_kchars[idx] & (self.rx_data.word_select(idx, 8) == 0x81)
<modwizcode>
mmm
<agg>
oh, oops, I left the Settle in the pastebin but it's not needed or meant to be there, you can delete L53
<modwizcode>
it doesn't need to be there?
<modwizcode>
I found it to be functional somewhat but perhaps it's not relevant without the prints
<modwizcode>
your version is a lot nicer though so thank you for looking at it :)
<agg>
I think once upon a time the plain yield turned into basically "settle, tick, settle"
<modwizcode>
yeah
<agg>
it makes a difference with the print as the print will observe the result of the comb logic updating from your input change, after a settle
<modwizcode>
the concern is when do values for the vcd get captured
<agg>
but doing it immediately before the tick doesn't have an effect
<agg>
in the vcd, the inputs you drive from the testbench appear to change on the rising edge, and are just _after_ the registers capture
<agg>
i.e. on each clock rising edge, the immediately previous value of inputs is captured, and the newly set value from a testbench appears
<agg>
this is the confusion that the issue talks about maybe having testbench changes take effect on the falling edge would help clarify
<agg>
in practice I mostly avoid using print or settle and instead do something like that testbench, capture values immediately after ticking (and assert on them)
<agg>
(works well with pytest, since it will print both sides of the assert on failure and so on)
<modwizcode>
okay that's basically what I thought
<modwizcode>
The assertions are more practical for validation but it helps to be able to conceptualize it, I rely on the VCDs a lot when it comes to looking at the data itself
<agg>
if you write a little testbench that has an input and an output and m.d.sync += output.eq(input) and in your testbench you just for idx in range(10): yield input.eq(idx); yield; and then check the vcd it might be clearer
<agg>
oh yea totally, I still generate a vcd from everything
<modwizcode>
I've not messed with pytest :p
<agg>
it's super super super easy
<agg>
that pastebin I did is valid
<modwizcode>
I just realized I got distracted by windows 98 vm and didn't finish reading it XD
<modwizcode>
oh can you feed that in without change?
<agg>
it detects the function called test_* and knows it should be a test, runs it, and passes if there are no assert failures
<modwizcode>
I wish it would print the successful asserts too (maybe it's a setting) it'd be really nice to (even for passes) print the signals on both side of a tick prior to an assert or something weird
<modwizcode>
Now that I say it, it sounds less generic than it seemed in my head
<agg>
in general it doesn't output on success because that would get messy fast, but it does capture prints and if the test fails it then shows the prints
<agg>
as well as the values on both sides of the assert, the current value of any arguments to the function, a snippet of the code, a traceback, etc
<agg>
and you can put extra arguments to the test function and "parameterise" it and pytest will call it with each of those values, which can be handy
<agg>
it does a lot, but it's nice that you don't need any extra stuff in your code if you don't want it, my file doesn't even import pytest or anything
<agg>
just has a method named test_aligner() and that's enough
<modwizcode>
yeah
<modwizcode>
I wonder how easy it'd be to write a tool to capture a traceback of updates to a signal printed on a failed assert
<agg>
just print the value on each step, heh
<modwizcode>
You could even potentially build the direct input tree and log those too
<agg>
pytest captures stdout during test execution and then displays it on failure
<modwizcode>
Yeah but automaticly generating it would make it useful
<agg>
(though i won't usually leave print statements in)
<agg>
yea, sure
<modwizcode>
Specially if you want to avoid exposing the consituent driver signals.
<agg>
pytest makes it easy to add custom formatters on failures, if you could also store the history in pysim you could dig it out and print it
<agg>
probably at that point you just wanna open the vcd though
<modwizcode>
I guess the work is writing a bit in nmigen that can auto capture the input history
<modwizcode>
eeeeh VCD's are so annoying and messy to try to work with imo
<agg>
storing the state of each signal on each clock probably isn't too bad, but I still think I'd want the extra context of the vcd
<modwizcode>
Usually all I care about are the signal and a few relevant inputs used to compute it
<agg>
you can tell nmigen to output a gtkw file that contains the traces you want to look at already inserted
<agg>
rather than having to drag and drop them each time, at least
<modwizcode>
that's true I guess. I suppose this would solve the "find the failure in the big VCD" problem
<agg>
so most of my tests output a vcd and a gtkw with the traces relevant to that test set up
<modwizcode>
like for this test sure it's tiny
<modwizcode>
but for other things I need to feed in an entire udp conversation before I see a failure.
<modwizcode>
At the very least it could dump enough info to speed up getting to the context in the VCD
hellsenberg has joined #nmigen
revolve has quit [Read error: Connection reset by peer]