00:18 <_whitenotifier> [YoWASP/nextpnr] whitequark pushed 1 commit to develop [+0/-0/±1] https://git.io/JIwQj

00:18 <_whitenotifier> [YoWASP/nextpnr] whitequark 366e8a0 - Update dependencies.

02:17 <awygle> good evening

07:32 <phire> What should I do when nmigen starts taking 3 min to generate verilog, and generating 26mb .v files?

07:33 <phire> is it a sign I should be writing certain inner parts directly in verilog?

07:35 <phire> or is it a bug/deficiency in the implemenation of nmigen

08:00 <whitequark> phire: it could be an inefficiency in your design as well

08:00 <whitequark> hard to say without context

08:00 <whitequark> are you using a lot of Arrays?

08:01 <phire> arrays and loops

08:01 <phire> I'm building a 64x64 bit matrix

08:01 <phire> and I build custom set logic for each bit

08:01 <whitequark> right

08:01 <whitequark> so, if you know how to implement this more efficiently in verilog, you should be able to do the same thing in nmigen

08:02 <phire> well in verilog, I'd use a for loop. I've used a for loop in nmigen too

08:03 <phire> but nmigen seems to choke processing the results of expanding the for loop

08:03 <whitequark> each array access is essentially a for loop on its own

08:04 <whitequark> so if you access an array in a loop, you're building a *lot* of logic

08:04 <daveshah> I think a synthesis tool is ultimately going to do similar transforms to Verilog, though

08:04 <phire> it seems to synthisize reasonably fine

08:05 <phire> quartus has no problems injecting the 26mb of verilog

08:05 <whitequark> yep, if you use SV arrays, the results would be similar

08:05 <daveshah> That strongly suggests it can be simplified, and that synthesis is optimising things

08:06 <whitequark> can you perhaps hoist some array accesses out of the loop using a temporary signal?

08:07 <phire> so this is my inner loop (one row of the matrix)

08:07 <phire> https://gist.github.com/phire/2ee44e1a0c72e7f64dcfc976c48304b0

08:07 <whitequark> ouch

08:07 <phire> num_values = 64, num_sets = 4, num_sets_per_row = 2

08:08 <whitequark> wait, in self.addr, do you even need the outer Array?

08:08 <phire> there are 4 "set ports" to the matrix, each set port can set two bits of a row

08:08 <whitequark> it could be a list, I think?

08:08 <phire> yeah, could probally be a list

08:09 <phire> I only statically index into it

08:10 <whitequark> same for row_set

08:10 <whitequark> this shouldn't actually affect the size of the verilog

08:10 <whitequark> but it makes intent more clear

08:10 <phire> yeah

08:27 <phire> whoops, now I'm crashing quartus_fit

08:30 <phire> (i fixed a bug in my code so quartus_map wasn't removing almost all the registers)

08:54 <phire> For all that crazyness, It synthesizes down to a pretty reasonable 2.03 ALUTs per register.

10:03 <phire> Ok, I optimised the code to be more nmigen friendly (I took inspration from the optimisations that synthesis did)

10:03 <phire> nmigen now generations 1.5mb of verilog in 8 seconds

16:22 <agg> would it be possible in pysim to measure 'coverage' of things like FSM states, if conditions true/false, etc, over a test/series of tests?

17:04 <whitequark> not at the moment, but it would be a reasonable extension

17:04 <whitequark> that said, sensibly expressing results is difficult

17:04 <whitequark> we could probably produce an SC/CC coverage file for ingestion to some standard tools...

17:11 <agg> maybe even write pycov-compatible outputs, then you could use pycov reporting and xml generation and everything else

17:11 <agg> huh, coverage.py, rather

17:13 <agg> not sure if you can use coverage.py directly (via either its api or its plugin interface) to add different coverage information, rather than just trying to write coverage-compatible outputs, which might be a bit cursed

17:14 <whitequark> coverage explicitly tells you to not write coverage-compatible outputs

17:15 <whitequark> idk, might be worth writing our own, then cxxrtl could use it as well

17:16 <agg> it looks like coverage file tracing plugins can 'claim' a python-or-otherwise file and report coverage for it, which could work for cxxsim, but wouldn't help cxxrtl I guess

17:18 <whitequark> cxxrtl can export coverage information via an API

17:18 <whitequark> so that's not an issue

19:01 <whitequark> awygle: poke?

19:02 <awygle> whitequark: peek

19:06 <whitequark> awygle: up for discussing cxxsim later today?

19:06 <awygle> sure

19:06 <awygle> just ping me whenever

19:06 <awygle> (well whenever within the next approx 12 hours)

19:07 <whitequark> ack

20:08 <whitequark> awygle: mkay, so i'd like to talk about simulation phases & triggering

20:12 <awygle> mk

20:37 <whitequark> awygle: sorry, got distracted

20:37 <whitequark> i think we actually discussed this earlier

20:37 <whitequark> do you recall if that was public or private?

21:03 <awygle> uhh

21:03 <awygle> i don't recall

21:03 <awygle> probably public? we don't talk privately that often

21:04 <awygle> whitequark: also sorry, was lunch

21:06 <whitequark> https://freenode.irclog.whitequark.org/nmigen/2020-09-02#1599015932-1599025611;

21:07 <awygle> oh yeah i remember that

21:08 <whitequark> ... i can't believe that was just 3 months ago, i thought it was in 2019

21:08 <awygle> i can't believe it was 3 entire months ago, i thought it was like, last week

21:09 <whitequark> time, huh

21:10 <awygle> yep

21:12 <awygle> so have your thoughts evolved since then?

21:12 <whitequark> they did, and so did the problems i'm facing

21:13 <whitequark> here's my current view of cxxrtl

21:14 <whitequark> cxxrtl generates a eval and commit function, but really it's generating two significantly distinct kinds of code that happen to be squished into the same boxes

21:17 <whitequark> first, it generates code that evaluates combinatorial cells. on a well-behaved netlist (i.e. without combinatorial loops) this code can be fully statically scheduled. we go to great lengths to turn not-quite-well-behaved netlists into entirely well-behaving ones, but if worst comes to worst, this code can be called in a loop until it settles.

21:18 <whitequark> but really, its intended purpose is to be executed once per simulation instant, and the current approach is actually a global performance optimum. nothing to do here.

21:19 <awygle> yep, makes sense

21:20 <whitequark> (this is basically my answer to your earlier "process per cell" suggestion)

21:20 <awygle> mhm

21:23 <whitequark> (there's an interesting detail here where the behavior of fully asynchronous logic / combinatorial loops is essentially undefined beyond "it's fully deterministic", but i digress)

21:31 <whitequark> second, it generates code that evaluates synchronous cells. this is a lot more tricky. scheduling-wise, this code can run at pretty much any order as long as it happens after all the combinatorial cells are evaluated. the trickiness is in the guarding condition.

21:38 <whitequark> the purpose of the guarding condition is pretty much to evaluate synchronous cells when (e.g.) there is a posedge on clk. that doesn't sound very complex. but the exact form of the condition has major ergonomics and performance implications in cxxrtl alone, and cxxsim makes all that twice as complex

21:40 <whitequark> let's look at cxxrtl alone first

21:42 <whitequark> how do we detect a posedge on clk? well, an obvious way to do so is during commit: we know the clock just changed, if it's 1 now, we have a posedge.

21:42 <whitequark> however, this requires the clock to be a wire<>.

21:44 <whitequark> so if you have code in a testbench like `top.p_clk.set(true); top.step();` then this will run eval/commit twice: first eval does nothing, first commit detects the edge, second eval/commit is actually useful.

21:45 <whitequark> that's a 2x performance hit which obviously sucks.

21:56 <whitequark> so what i do instead is one of two things:

21:57 <whitequark> - if p_clk is a wire<> after all, i compare p_clk.curr and p_clk.next to see if it's about to be committed, and if it is, i consider that a posedge has happened.

21:58 <whitequark> this is strictly speaking illegal according to my own rules, and unsurprisingly, it causes issues i'll go into later

21:59 <whitequark> - if p_clk is a value<>, i create another value<> prev_p_clk, do `prev_p_clk = p_clk` in commit, and compare them to determine if a posedge has happened.

22:09 <whitequark> so, what goes wrong here?

22:29 <awygle> got distracted oops

22:31 <awygle> the former sounds like it could cause issues if the combinatorial logic isn't settled, for one thing

22:58 <whitequark> awygle: sorry, got distracted

22:58 <whitequark> oh, you too

22:58 <whitequark> ok, so you're right, that is one issue

23:03 <whitequark> another issue is... actually hang on i found a bug

23:39 <whitequark> https://github.com/YosysHQ/yosys/pull/2485

23:40 <whitequark> awygle: ok, so other than the issue you pointed out, i'm not aware of how this plan breaks. it's just gross.

23:41 <whitequark> (i thought it would break, but it turned out to be a simple miscompilation that i fixed in #2485)

23:47 <whitequark> awygle: still here?