Monday, 2018-04-16

*** tpb has joined #vtr-dev00:00
*** digshadow has quit IRC07:11
*** digshadow has joined #vtr-dev07:11
mithrojhol: Morning?14:11
jholmithro: hi - how's it going?14:21
mithrojhol: Not too bad -- did you see the stuff I did over the weekend?14:22
jholyeah good stuff, but a nasty divergence14:22
jholjust pulling my PLB work back together14:22
mithrojhol: Oh?14:22
jholon my resturecture-rabase I had wrapped the 8 LUTFFs in a pb_type called PLB_LOCAL14:23
jholthis pb_type is the thing that will get the SB_DFF modes14:23
jholyou've moved the PLB out to the devices/14:23
jholso I'm just re-wrapping the PLB core into PLB_LOCAL, so the modes can fit in14:24
mithrojhol: Did you see my comments about not needing to do that?14:25
jholno I didn't see that14:25
jholwhat was the argument?14:25
mithrojhol: The packer will only pack things together which the routing allows14:26
mithrojhol: So it should reject packing together two different FF modes because it should cause a routing conflict14:27
jholhmm - that's a good point, but is that really sufficient?14:27
mithrojhol: Sure? Why wouldn't it be?14:27
jhola SR_DFFSS has the same pins as a SR_DFFS IIRC14:27
jholor does it.... let me see14:27
mithrojhol: Maybe I'm wrong :-P14:27
jholit does14:28
mithroLet me see...14:28
jholit's described in the ICE Technology Library document14:28
mithrojhol: Where are the bits that describe the SR_DFFS and SR_DFFSS in icestorm?14:32
jholjust searching14:33
mithroLC_i[8] is the CarryEnable bit. This bit must be set if the carry logic is used.14:35
mithroLC_i[9] is the DffEnable bit. It enables the output flip-flop for the LUT.14:35
mithroLC_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
mithroLC_i[19] is the AsyncSetReset bit. When this bit is set then the set/reset signal is asynchronous to the clock.14:35
jholand NegClk14:37
mithrojhol: So for SR_DFFS the AsyncSetReset must be low, while for the SR_DFFSS the AsyncSetReset must be high14:37
jholyeah14:37
mithrojhol: That sounds like a routing conflict to me?14:37
jholright... but you're not going to be able to model that bit with a mux, right?14:38
jholVPR doesn't understand such things14:39
jholwith 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 to14:40
mithrojhol: 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
mithroThat is my understanding14:44
mithroBut the best way is to test and see14:45
mithroI'm guessing that you went down this path because the packer was doing something wrong?14:45
jholyeah it was putting the 3 DFFs of my 3-bit counter together in the same tile14:46
jholI'm pretty close to having it done, it's just a big change that's vulnerable to any PLB changes you're making14:48
mithroDo you have you example and some output I can look at?14:48
mithroI feel like this is probably just a simple issue in the clb description...14:49
mithrojhol: do you know about turning on the .echo file output?14:50
jholno what's that?14:50
mithroIt give you a lot of info about what vpr is doing internally14:59
mithroGo into the tests directory and type "make wire.echo"15:00
jholok, I'll take a look15:01
jholI noticed SR_DFFES was missing, so I added it: https://github.com/jhol/symbiflow-arch-defs/commit/6049ef2760e77410fd220a2aab01010574eb34e915:02
tpbTitle: ice40/primitivis/sb_ff: Added SB_DFFES model · jhol/[email protected] · GitHub (at github.com)15:02
jholI 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
mithrojhol: Unclear :-)15:04
jholok... not going down that rabbit hole for now15:05
mithrojhol: Do you have a patch which adds your counter to the tests?15:06
mithrojhol: I would like to reproduce the issue you are having15:08
jholyeah here: https://github.com/jhol/symbiflow-arch-defs/commit/c35dcf76ff00ae475859fcf426a47e5305ea064415:10
tpbTitle: tests: Added a 3-bit counter test · jhol/[email protected] · GitHub (at github.com)15:10
mithrodeclare -x ARCH="ice40" ?15:13
mithrohttps://www.irccloud.com/pastebin/5UQPxzgh/15:14
tpbTitle: Snippet | IRCCloud (at www.irccloud.com)15:14
jholyeah - you're getting the 2x SR_DFF and 1x SR_DFFE - they have to be in 2 separate tiles15:16
mithrohttps://www.irccloud.com/pastebin/tnTInWQx/15:16
tpbTitle: Snippet | IRCCloud (at www.irccloud.com)15:16
mithrojhol: That comes from "atom_netlist.orig.echo.blif"15:19
mithrojhol: So, you are correct -- the current way of specifying the SB_FF's means that they will get packed together15:21
jholyeah - well I'm almost finished with my rafactor to fix it15:22
mithrojhol: Two options, go to tile level modes, or change the SB_DFF to contain the configuration properties15:22
jholmithro: is it me, or is the carry chain backwards in ice40/devices/top-routing-virt/tiles/plb/plb.pb_type.xml15:32
mithrojhol: I thought I fixed that?15:33
jholit goes the other way in tile-routing-virt15:33
jholdo you know which way is correct?15:33
mithroNope!15:35
jhol\o/ !15:35
jholI assumed it was FCIN -> 0 -> 1 ... -> 7 -> FCOUT15:35
jhol:w16:14
mithrojhol: !?16:17
jholhi!16:28
jholmithro: hi16:30
mithrojhol: Success?16:30
jholit's the moment of truth16:30
jholsquashing bugs16:35
*** digshadow has quit IRC16:40
jholmithro: here's my branch for you to inspect: https://github.com/jhol/symbiflow-arch-defs/commits/modes-fixes16:40
tpbTitle: Commits · jhol/symbiflow-arch-defs · GitHub (at github.com)16:40
jholit crashes VPR at the moment16:40
jholI had a similar problem previously, though, and I found a hack around it16:40
mithrojhol: Where is it segfaulting?16:41
jholhttps://paste2.org/ndzKKP9B16:42
jholthink it's related to carry chain mollecules16:42
mithrojhol: checking now...16:46
jholI'll explain the change: basically, I wrapped 8xLUTFF array with a pb_type: PLB_LOCAL16:49
jholthis is then shared between the tiled and top versions of the PLB16:49
jholPLB_LOCAL is needed, because for every <mode>, you have to redefine the complete contents including the nested pb_types and the interconnect16:50
jholparticularly for the tile-routing version, this would mean a huge amount of repetition for every mode16:50
mithrojhol: Yeah - I get the theory - really this should be called "BLK_IG-PLB_MODE" or something ?16:50
jholyeah I'm not to bothered by the exact name16:51
jholanyway... I'm keen to get these changes integrated, because it's been an absolute pig to deal with divergence16:52
mithrojhol: I wonder about having the same names inside of each of the modes -- but I guess it must be okay?16:52
jholyes I wondered about that16:52
jhol- not sure16:52
mithrobtw - you should first look at the arch.echo16:52
jholgood idea16:52
mithroand check that looks like what you expect16:52
mithrojhol: So I get a segfault in mostly the same area16:54
mithroWarning 220: lutff_global[0].i_EN[0] unconnected pin in architecture.16:55
mithroWarning 221: lutff_global[0].i_SR[0] unconnected pin in architecture.16:55
mithroWarning 222: lutff_global[0].o_EN[0] unconnected pin in architecture.16:55
mithroWarning 223: lutff_global[0].o_CLK[0] unconnected pin in architecture.16:55
mithroWarning 224: lutff_global[0].o_SR[0] unconnected pin in architecture.16:55
mithroWarning 225: lutff_global[0].i_CLK[0] unconnected pin in architecture.16:55
jholyeah I see that16:55
mithrojhol: Do you expect that?16:55
jholyeah I see that16:56
jholthe top routing version is simpler16:56
mithrojhol: Yeah - I would start with that when playing with the internals of the tile16:57
mithrokem_: 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
mithrokem_: I assume not?16:57
jholmithro: 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 variants16:58
jholI can fix that... it will just make it more verbose16:58
mithrojhol: I don't think that really matters...16:58
mithrojhol: pb_graph.echo and lb_type_rr_graph.echo could also provide insight16:59
mithrojhol: Also recommend using the 'wire' input when debugging16:59
mithrojhol: 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
jholnice17:01
mithrojhol: Looks like the failure is that root_block is a null pointer17:02
mithrojhol: /* load packing patterns by traversing the edges to find edges belonging to pattern */17:03
mithroLimitations: 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
mithroFrom the comment to that function....17:04
mithrojhol: I'm unsure what populates the root_block of a pack pattern17:05
jholmaybe it's to do with the labeling of FCOUT and FCOUT717:05
jholthey fan out, but FCOUT7, doesn't need the direct tag17:06
mithrostatic void backward_expand_pack_pattern_from_edge(17:06
jholjust 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 up17:06
jholnot sure if there's a way to leave a port dangling17:07
mithrojhol: 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 nods17:08
mithrojhol: It might be enough to just check for null in that if...17:09
mithrojhol: Giving that a go17:09
jholok... just got past the segfault17:10
mithrojhol: Okay, that fix gives me a useful error message :-P17:11
mithroError 1:17:12
mithroType: Architecture file17:12
mithroFile: /usr/local/google/home/tansell/work/catx/vtr-verilog-to-routing/vpr/src/pack/prepack.cpp17:12
mithroLine: 13917:12
mithroMessage: Failed to find root block for pack pattern CARRYCHAIN17:12
jhol.https://github.com/jhol/symbiflow-arch-defs/commit/9f9b5441d27454c8aa1ee621116a041e09cb919917:12
tpbTitle: HACK: Around a segfault in the packer · jhol/[email protected] · GitHub (at github.com)17:12
jholnow it's putting the architecture into three tiles17:12
jholI'm using the display mode to look at this: make ARCH=ice40 DEVICE_TYPE=top-routing-virt DEVICE=test4 VPR_ARGS='--disp on' counter.gdb17:13
jholI think it should have been able to get the design into 2 tiles17:13
jholone thing I noticed earlier is that the "lout" signal doesn't have the necessary muxes17:14
jholI added the lout pin to the LUTFF, but didn't add any muxes to the PLB yet17:14
jholthat might prevent it from packing the mixture of SB_DFFs, SB_CARRYs and SB_LUT4s as well it should do17:16
mithrojhol: The render is pretty unhappy with the flip flops drawing17:16
jholI never got the render thing to work17:16
jholhow does it do the render?17:16
jholgraphviz?17:16
mithrohttps://usercontent.irccloud-cdn.com/file/0SH1z2rB/image.png17:17
mithrojhol: I'm talking vpr?17:17
jholyeah - that's what I get17:17
jhol-- I was referring to the "make render" rule17:18
mithromake render is unrelated to vpr17:18
jholok ok17:18
mithrojhol: You need a pack pattern for it to pair ff and luts together properly17:18
jholright ok17:19
jholfor the lut-ff bridge, there was one previously, but it was commented out17:19
mithrohttps://docs.verilogtorouting.org/en/latest/arch/reference/#tag-interconnect-pack_pattern17:19
mithroSee the last example17:20
jholyeah17:20
jholalso, while I remember: does VPR have any way to do pin constraints?17:20
jholat the moment it seems to pick the pins at random17:20
mithrojhol: I believe so17:21
jholI tried looking for a command-line argument, and didn't see anything - yet17:21
mithro--fix_pins {random | <file.pads>}17:22
mithroThis 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
mithrojhol: Oh also "--debug_clustering {on | off}"17:24
jholoooh nice17:24
mithrojhol: Controls verbose clustering output (useful for debugging architecture packing problems).17:24
mithroJust discovered that then17:24
mithrobut I do think kem_ linked me to it previously17:24
mithroI'm going to head out to grab coffee, will be back in a bit17:25
jholyeah ok - chat with you at the meeting17:27
jholinteresting... got a similar error when trying to apply <pack_pattern> to LUT2FF: "Message: Failed to find root block for pack pattern LUT2FF"17:35
jholso I suppose it doesn't like pack_patterns that don't involve the root tile element in some way17:35
jholthat's annoying17:36
jholall the pack_patterns have to come out in the root tile's <interconnect>17:36
*** digshadow has joined #vtr-dev17:41
mithrojhol: That is my understanding....17:46
mithrojhol: 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
mithrodigshadow: Morning17:48
digshadowmithro: hi17:50
mithrodigshadow: jhol is making some progress on getting the stuff to pnr on virtual fabric17:52
digshadownice17:55
digshadowmithro: did you poke at stuff over the weekend17:56
mithrodigshadow: Yes17:56
mithrodigshadow: I don't think I poked at anything which should affect your stuff17:56
digshadowokay17:56
mithrodigshadow: But I would rebase anyway17:56
digshadowmithro: against origin/master?18:04
mithrodigshadow: Against jhol's changes probably...18:05
digshadowmithro: did you by chance merge in the vtr thing18:05
mithroWhich vtr thing?18:05
mithrojhol: 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
mithrokem_: 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 implementations19: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
mithrokem_: Okay19:14
mithroI have some things for drawing an rr_graph using graphviz -- so was hoping I could draw an internal tile rr_graph using the same stuff19:15
mithroBut for now I guess not19: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 with19:21
mithrokem_: Yeah19:22
mithrokem_: Just need to do some regexing :-P19:28
mithrokem_: I believe digshadow will be sending a pull request for a change in the rr_graph format19:45
mithrojhol: FYI -> https://github.com/verilog-to-routing/vtr-verilog-to-routing/issues/31819:51
tpbTitle: Assert failure when writing atom_netlist.cleaned.echo.blif · Issue #318 · verilog-to-routing/vtr-verilog-to-routing · GitHub (at github.com)19:51
digshadowmithro: I was in process of making it I just realized there were more debug prints20:01
digshadowshould be coming shortly though20:01
mithrokem_: Feel free to reach out to digshadow if you have questions about why we needed this20:02
digshadoweh I need to update the docs as well20:06
kem_digshadow: At first glance the PR seems reasonable. I'll try to take a look in detail tomorrow.20:46
mithrojhol: Did you disappear?22:57

Generated by irclog2html.py 2.13.1 by Marius Gedminas - find it at mg.pov.lt!