*** tpb has joined #vtr-dev | 00:00 | |
*** digshadow has quit IRC | 07:11 | |
*** digshadow has joined #vtr-dev | 07:11 | |
mithro | jhol: Morning? | 14:11 |
---|---|---|
jhol | mithro: hi - how's it going? | 14:21 |
mithro | jhol: Not too bad -- did you see the stuff I did over the weekend? | 14:22 |
jhol | yeah good stuff, but a nasty divergence | 14:22 |
jhol | just pulling my PLB work back together | 14:22 |
mithro | jhol: Oh? | 14:22 |
jhol | on my resturecture-rabase I had wrapped the 8 LUTFFs in a pb_type called PLB_LOCAL | 14:23 |
jhol | this pb_type is the thing that will get the SB_DFF modes | 14:23 |
jhol | you've moved the PLB out to the devices/ | 14:23 |
jhol | so I'm just re-wrapping the PLB core into PLB_LOCAL, so the modes can fit in | 14:24 |
mithro | jhol: Did you see my comments about not needing to do that? | 14:25 |
jhol | no I didn't see that | 14:25 |
jhol | what was the argument? | 14:25 |
mithro | jhol: The packer will only pack things together which the routing allows | 14:26 |
mithro | jhol: So it should reject packing together two different FF modes because it should cause a routing conflict | 14:27 |
jhol | hmm - that's a good point, but is that really sufficient? | 14:27 |
mithro | jhol: Sure? Why wouldn't it be? | 14:27 |
jhol | a SR_DFFSS has the same pins as a SR_DFFS IIRC | 14:27 |
jhol | or does it.... let me see | 14:27 |
mithro | jhol: Maybe I'm wrong :-P | 14:27 |
jhol | it does | 14:28 |
mithro | Let me see... | 14:28 |
jhol | it's described in the ICE Technology Library document | 14:28 |
mithro | jhol: Where are the bits that describe the SR_DFFS and SR_DFFSS in icestorm? | 14:32 |
jhol | just searching | 14:33 |
mithro | LC_i[8] is the CarryEnable bit. This bit must be set if the carry logic is used. | 14:35 |
mithro | LC_i[9] is the DffEnable bit. It enables the output flip-flop for the LUT. | 14:35 |
mithro | LC_i[18] is the Set_NoReset bit. When this bit is set then the set/reset signal will set, not reset the flip-flop. | 14:35 |
mithro | LC_i[19] is the AsyncSetReset bit. When this bit is set then the set/reset signal is asynchronous to the clock. | 14:35 |
jhol | and NegClk | 14:37 |
mithro | jhol: So for SR_DFFS the AsyncSetReset must be low, while for the SR_DFFSS the AsyncSetReset must be high | 14:37 |
jhol | yeah | 14:37 |
mithro | jhol: That sounds like a routing conflict to me? | 14:37 |
jhol | right... but you're not going to be able to model that bit with a mux, right? | 14:38 |
jhol | VPR doesn't understand such things | 14:39 |
jhol | with the modes things, it's going to emit a list of tile configs and their routes, and a bitstream packer would simply look at which tile mode was selected, to decide what state to set that bit to | 14:40 |
mithro | jhol: vpr would see that in the SR_DFFS case that pin needs to be driven with constant high and the SR_DFF case the pin needs to be driven low, as there is no way inside a logic tile to allow different FF to have a different pin levels it shouldn't pack them together... | 14:44 |
mithro | That is my understanding | 14:44 |
mithro | But the best way is to test and see | 14:45 |
mithro | I'm guessing that you went down this path because the packer was doing something wrong? | 14:45 |
jhol | yeah it was putting the 3 DFFs of my 3-bit counter together in the same tile | 14:46 |
jhol | I'm pretty close to having it done, it's just a big change that's vulnerable to any PLB changes you're making | 14:48 |
mithro | Do you have you example and some output I can look at? | 14:48 |
mithro | I feel like this is probably just a simple issue in the clb description... | 14:49 |
mithro | jhol: do you know about turning on the .echo file output? | 14:50 |
jhol | no what's that? | 14:50 |
mithro | It give you a lot of info about what vpr is doing internally | 14:59 |
mithro | Go into the tests directory and type "make wire.echo" | 15:00 |
jhol | ok, I'll take a look | 15:01 |
jhol | I noticed SR_DFFES was missing, so I added it: https://github.com/jhol/symbiflow-arch-defs/commit/6049ef2760e77410fd220a2aab01010574eb34e9 | 15:02 |
tpb | Title: ice40/primitivis/sb_ff: Added SB_DFFES model · jhol/symbiflow-arch-defs@6049ef2 · GitHub (at github.com) | 15:02 |
jhol | I also noticed that none of the SR_DFN primitives are listed - but I assume that's because there's no way for VPR to understand different clock phase? | 15:03 |
mithro | jhol: Unclear :-) | 15:04 |
jhol | ok... not going down that rabbit hole for now | 15:05 |
mithro | jhol: Do you have a patch which adds your counter to the tests? | 15:06 |
mithro | jhol: I would like to reproduce the issue you are having | 15:08 |
jhol | yeah here: https://github.com/jhol/symbiflow-arch-defs/commit/c35dcf76ff00ae475859fcf426a47e5305ea0644 | 15:10 |
tpb | Title: tests: Added a 3-bit counter test · jhol/symbiflow-arch-defs@c35dcf7 · GitHub (at github.com) | 15:10 |
mithro | declare -x ARCH="ice40" ? | 15:13 |
mithro | https://www.irccloud.com/pastebin/5UQPxzgh/ | 15:14 |
tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 15:14 |
jhol | yeah - you're getting the 2x SR_DFF and 1x SR_DFFE - they have to be in 2 separate tiles | 15:16 |
mithro | https://www.irccloud.com/pastebin/tnTInWQx/ | 15:16 |
tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 15:16 |
mithro | jhol: That comes from "atom_netlist.orig.echo.blif" | 15:19 |
mithro | jhol: So, you are correct -- the current way of specifying the SB_FF's means that they will get packed together | 15:21 |
jhol | yeah - well I'm almost finished with my rafactor to fix it | 15:22 |
mithro | jhol: Two options, go to tile level modes, or change the SB_DFF to contain the configuration properties | 15:22 |
jhol | mithro: is it me, or is the carry chain backwards in ice40/devices/top-routing-virt/tiles/plb/plb.pb_type.xml | 15:32 |
mithro | jhol: I thought I fixed that? | 15:33 |
jhol | it goes the other way in tile-routing-virt | 15:33 |
jhol | do you know which way is correct? | 15:33 |
mithro | Nope! | 15:35 |
jhol | \o/ ! | 15:35 |
jhol | I assumed it was FCIN -> 0 -> 1 ... -> 7 -> FCOUT | 15:35 |
jhol | :w | 16:14 |
mithro | jhol: !? | 16:17 |
jhol | hi! | 16:28 |
jhol | mithro: hi | 16:30 |
mithro | jhol: Success? | 16:30 |
jhol | it's the moment of truth | 16:30 |
jhol | squashing bugs | 16:35 |
*** digshadow has quit IRC | 16:40 | |
jhol | mithro: here's my branch for you to inspect: https://github.com/jhol/symbiflow-arch-defs/commits/modes-fixes | 16:40 |
tpb | Title: Commits · jhol/symbiflow-arch-defs · GitHub (at github.com) | 16:40 |
jhol | it crashes VPR at the moment | 16:40 |
jhol | I had a similar problem previously, though, and I found a hack around it | 16:40 |
mithro | jhol: Where is it segfaulting? | 16:41 |
jhol | https://paste2.org/ndzKKP9B | 16:42 |
jhol | think it's related to carry chain mollecules | 16:42 |
mithro | jhol: checking now... | 16:46 |
jhol | I'll explain the change: basically, I wrapped 8xLUTFF array with a pb_type: PLB_LOCAL | 16:49 |
jhol | this is then shared between the tiled and top versions of the PLB | 16:49 |
jhol | PLB_LOCAL is needed, because for every <mode>, you have to redefine the complete contents including the nested pb_types and the interconnect | 16:50 |
jhol | particularly for the tile-routing version, this would mean a huge amount of repetition for every mode | 16:50 |
mithro | jhol: Yeah - I get the theory - really this should be called "BLK_IG-PLB_MODE" or something ? | 16:50 |
jhol | yeah I'm not to bothered by the exact name | 16:51 |
jhol | anyway... I'm keen to get these changes integrated, because it's been an absolute pig to deal with divergence | 16:52 |
mithro | jhol: I wonder about having the same names inside of each of the modes -- but I guess it must be okay? | 16:52 |
jhol | yes I wondered about that | 16:52 |
jhol | - not sure | 16:52 |
mithro | btw - you should first look at the arch.echo | 16:52 |
jhol | good idea | 16:52 |
mithro | and check that looks like what you expect | 16:52 |
mithro | jhol: So I get a segfault in mostly the same area | 16:54 |
mithro | Warning 220: lutff_global[0].i_EN[0] unconnected pin in architecture. | 16:55 |
mithro | Warning 221: lutff_global[0].i_SR[0] unconnected pin in architecture. | 16:55 |
mithro | Warning 222: lutff_global[0].o_EN[0] unconnected pin in architecture. | 16:55 |
mithro | Warning 223: lutff_global[0].o_CLK[0] unconnected pin in architecture. | 16:55 |
mithro | Warning 224: lutff_global[0].o_SR[0] unconnected pin in architecture. | 16:55 |
mithro | Warning 225: lutff_global[0].i_CLK[0] unconnected pin in architecture. | 16:55 |
jhol | yeah I see that | 16:55 |
mithro | jhol: Do you expect that? | 16:55 |
jhol | yeah I see that | 16:56 |
jhol | the top routing version is simpler | 16:56 |
mithro | jhol: Yeah - I would start with that when playing with the internals of the tile | 16:57 |
mithro | kem_: Is there a way to get vpr to dump the local block internal rr_graph into a rr_graph.xml file rather than the "lb_type_rr_graph.echo" ? | 16:57 |
mithro | kem_: I assume not? | 16:57 |
jhol | mithro: ok... I know why... it's because I'm declaring all the the pins on each LUTFF e.g. EN, SR, but they don't get used in some variants | 16:58 |
jhol | I can fix that... it will just make it more verbose | 16:58 |
mithro | jhol: I don't think that really matters... | 16:58 |
mithro | jhol: pb_graph.echo and lb_type_rr_graph.echo could also provide insight | 16:59 |
mithro | jhol: Also recommend using the 'wire' input when debugging | 16:59 |
mithro | jhol: Kind of looks like an interaction between modes and the packing patterns.... | 17:00 |
mithro | $2 = {name = 0x5555568a6e50 "CARRYCHAIN", index = 0, root_block = 0x0, base_cost = 352, num_blocks = 88, is_block_optional = 0x5555568add70, is_chain = true, chain_root_pin = 0x55555643cd70} | 17:01 |
jhol | nice | 17:01 |
mithro | jhol: Looks like the failure is that root_block is a null pointer | 17:02 |
mithro | jhol: /* load packing patterns by traversing the edges to find edges belonging to pattern */ | 17:03 |
mithro | Limitations: Currently assumes that forced pack nets must be single-fanout as this covers all the reasonable architectures we wanted. | 17:04 |
mithro | More complicated structures should probably be handled either downstream (general packing) or upstream (in tech mapping). | 17:04 |
mithro | From the comment to that function.... | 17:04 |
mithro | jhol: I'm unsure what populates the root_block of a pack pattern | 17:05 |
jhol | maybe it's to do with the labeling of FCOUT and FCOUT7 | 17:05 |
jhol | they fan out, but FCOUT7, doesn't need the direct tag | 17:06 |
mithro | static void backward_expand_pack_pattern_from_edge( | 17:06 |
jhol | just tried to squash those warnings... not sure what the correct remedy is, because you can't make EN/SR disappear in some modes, because that will make the outer interconnect fail to connect up | 17:06 |
jhol | not sure if there's a way to leave a port dangling | 17:07 |
mithro | jhol: I think we probably want to put in a feature request for a way to explicitly tag a dangling port as valid? | 17:07 |
* jhol nods | 17:08 | |
mithro | jhol: It might be enough to just check for null in that if... | 17:09 |
mithro | jhol: Giving that a go | 17:09 |
jhol | ok... just got past the segfault | 17:10 |
mithro | jhol: Okay, that fix gives me a useful error message :-P | 17:11 |
mithro | Error 1: | 17:12 |
mithro | Type: Architecture file | 17:12 |
mithro | File: /usr/local/google/home/tansell/work/catx/vtr-verilog-to-routing/vpr/src/pack/prepack.cpp | 17:12 |
mithro | Line: 139 | 17:12 |
mithro | Message: Failed to find root block for pack pattern CARRYCHAIN | 17:12 |
jhol | .https://github.com/jhol/symbiflow-arch-defs/commit/9f9b5441d27454c8aa1ee621116a041e09cb9199 | 17:12 |
tpb | Title: HACK: Around a segfault in the packer · jhol/symbiflow-arch-defs@9f9b544 · GitHub (at github.com) | 17:12 |
jhol | now it's putting the architecture into three tiles | 17:12 |
jhol | I'm using the display mode to look at this: make ARCH=ice40 DEVICE_TYPE=top-routing-virt DEVICE=test4 VPR_ARGS='--disp on' counter.gdb | 17:13 |
jhol | I think it should have been able to get the design into 2 tiles | 17:13 |
jhol | one thing I noticed earlier is that the "lout" signal doesn't have the necessary muxes | 17:14 |
jhol | I added the lout pin to the LUTFF, but didn't add any muxes to the PLB yet | 17:14 |
jhol | that might prevent it from packing the mixture of SB_DFFs, SB_CARRYs and SB_LUT4s as well it should do | 17:16 |
mithro | jhol: The render is pretty unhappy with the flip flops drawing | 17:16 |
jhol | I never got the render thing to work | 17:16 |
jhol | how does it do the render? | 17:16 |
jhol | graphviz? | 17:16 |
mithro | https://usercontent.irccloud-cdn.com/file/0SH1z2rB/image.png | 17:17 |
mithro | jhol: I'm talking vpr? | 17:17 |
jhol | yeah - that's what I get | 17:17 |
jhol | -- I was referring to the "make render" rule | 17:18 |
mithro | make render is unrelated to vpr | 17:18 |
jhol | ok ok | 17:18 |
mithro | jhol: You need a pack pattern for it to pair ff and luts together properly | 17:18 |
jhol | right ok | 17:19 |
jhol | for the lut-ff bridge, there was one previously, but it was commented out | 17:19 |
mithro | https://docs.verilogtorouting.org/en/latest/arch/reference/#tag-interconnect-pack_pattern | 17:19 |
mithro | See the last example | 17:20 |
jhol | yeah | 17:20 |
jhol | also, while I remember: does VPR have any way to do pin constraints? | 17:20 |
jhol | at the moment it seems to pick the pins at random | 17:20 |
mithro | jhol: I believe so | 17:21 |
jhol | I tried looking for a command-line argument, and didn't see anything - yet | 17:21 |
mithro | --fix_pins {random | <file.pads>} | 17:22 |
mithro | This pad location file is in the same format as a normal placement file, but only specifies the locations of I/O pads, rather than the locations of all blocks. | 17:23 |
mithro | jhol: Oh also "--debug_clustering {on | off}" | 17:24 |
jhol | oooh nice | 17:24 |
mithro | jhol: Controls verbose clustering output (useful for debugging architecture packing problems). | 17:24 |
mithro | Just discovered that then | 17:24 |
mithro | but I do think kem_ linked me to it previously | 17:24 |
mithro | I'm going to head out to grab coffee, will be back in a bit | 17:25 |
jhol | yeah ok - chat with you at the meeting | 17:27 |
jhol | interesting... got a similar error when trying to apply <pack_pattern> to LUT2FF: "Message: Failed to find root block for pack pattern LUT2FF" | 17:35 |
jhol | so I suppose it doesn't like pack_patterns that don't involve the root tile element in some way | 17:35 |
jhol | that's annoying | 17:36 |
jhol | all the pack_patterns have to come out in the root tile's <interconnect> | 17:36 |
*** digshadow has joined #vtr-dev | 17:41 | |
mithro | jhol: That is my understanding.... | 17:46 |
mithro | jhol: I'm not sure why its not doing the packing without needing pack_patterns, it seems like it would be the lowest "cost" solution either way? | 17:47 |
mithro | digshadow: Morning | 17:48 |
digshadow | mithro: hi | 17:50 |
mithro | digshadow: jhol is making some progress on getting the stuff to pnr on virtual fabric | 17:52 |
digshadow | nice | 17:55 |
digshadow | mithro: did you poke at stuff over the weekend | 17:56 |
mithro | digshadow: Yes | 17:56 |
mithro | digshadow: I don't think I poked at anything which should affect your stuff | 17:56 |
digshadow | okay | 17:56 |
mithro | digshadow: But I would rebase anyway | 17:56 |
digshadow | mithro: against origin/master? | 18:04 |
mithro | digshadow: Against jhol's changes probably... | 18:05 |
digshadow | mithro: did you by chance merge in the vtr thing | 18:05 |
mithro | Which vtr thing? | 18:05 |
mithro | jhol: So, I guess we should have tests for lut->ff and ff->ff? | 18:49 |
kem_ | mithro: To clarify, the lb_type_rr_graph only dumps to its .echo format. | 19:12 |
mithro | kem_: Is the rr_graph in the lb in a similar format to the top layer rr_graph? Or do they just share a name? | 19:13 |
kem_ | mithro: They are similar in concept (a routing resource graph), but use different code implementations | 19:14 |
kem_ | mithro: Its a long term goal to merge them into a unified RR graph. But that requires re-writting a lot of the packer... | 19:14 |
mithro | kem_: Okay | 19:14 |
mithro | I have some things for drawing an rr_graph using graphviz -- so was hoping I could draw an internal tile rr_graph using the same stuff | 19:15 |
mithro | But for now I guess not | 19:15 |
kem_ | mithro: AFAIK the concepts should map pretty much 1:1 so it should be mostly a format conversion. But that's a part of the code-base I'm not too familiar with | 19:21 |
mithro | kem_: Yeah | 19:22 |
mithro | kem_: Just need to do some regexing :-P | 19:28 |
mithro | kem_: I believe digshadow will be sending a pull request for a change in the rr_graph format | 19:45 |
mithro | jhol: FYI -> https://github.com/verilog-to-routing/vtr-verilog-to-routing/issues/318 | 19:51 |
tpb | Title: Assert failure when writing atom_netlist.cleaned.echo.blif · Issue #318 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com) | 19:51 |
digshadow | mithro: I was in process of making it I just realized there were more debug prints | 20:01 |
digshadow | should be coming shortly though | 20:01 |
mithro | kem_: Feel free to reach out to digshadow if you have questions about why we needed this | 20:02 |
digshadow | eh I need to update the docs as well | 20:06 |
kem_ | digshadow: At first glance the PR seems reasonable. I'll try to take a look in detail tomorrow. | 20:46 |
mithro | jhol: Did you disappear? | 22:57 |
Generated by irclog2html.py 2.13.1 by Marius Gedminas - find it at mg.pov.lt!