| *** tpb has joined #vtr-dev | 00:00 | |
| *** vtrbot has joined #vtr-dev | 00:04 | |
| *** digshadow has quit IRC | 00:58 | |
| *** digshadow has joined #vtr-dev | 02:18 | |
| *** digshadow has quit IRC | 04:58 | |
| *** digshadow has joined #vtr-dev | 06:58 | |
| *** digshadow has quit IRC | 11:38 | |
| mithro | morning everyone! | 13:41 |
|---|---|---|
| mithro | kem_: Should my example arch+wire.eblif example work with latest head of vtr? I'm getting a different assertion failure now... | 13:42 |
| mithro | vtr-verilog-to-routing/vpr/src/place/timing_place_lookup.cpp:167 get_best_class: Assertion 'best_class >= 0 && best_class < type->num_class' failed. | 13:42 |
| kem_ | mithro: It did work for me, but I've also seen that failure when I was trying to make a regression test to cover the original issue | 13:43 |
| mithro | kem_: I've been meaning to ask how I should be creating tests which fit into the vtr system | 13:44 |
| kem_ | mithro: I added instructions to README.developers.md a couple weeks ago which give an overview | 13:45 |
| mithro | kem_: Okay great, I'll take a look at it | 13:45 |
| daveshah | kem_ and mithro: WRT https://github.com/mithro/vtr-verilog-to-routing/issues/5, where would be a good place to output attributes on locations? It looks like the .place file is the only option, but I'm not sure how useful that would be, or even how easy given the simple format of this file? | 13:45 |
| mithro | daveshah: I would say the place file or the output verilog | 13:47 |
| kem_ | mithro and daveshah: Its not entirely clear to me if tagging location attributes directly in VPR is the best approach. Can you give some insight into the motivation? | 13:49 |
| mithro | kem_: One big one is to able to tag the info which makes it possible to map vtr grid locations to real tile grid locations | 13:51 |
| mithro | kem_: but I can imagine in the future wanting vtr to do things based on the attributes -- for example I can imagine wanting to tag a block with "use only for route through" | 13:52 |
| mithro | kem_: I can imagine wanting it in the output verilog somehow | 13:54 |
| daveshah | At the moment no placement information is dumped into the Verilog. It would be great to add this (optionally, in case older tools don't support attributes) as Verilog attributes too | 13:54 |
| daveshah | It would also be moving towards allowing placement constraints in the long term | 13:55 |
| kem_ | I completely buy the idea of location constraints. (I've actually drafted up a format to that end previously). | 13:56 |
| kem_ | But I think those are really 'design constraints', rather than 'architecture/device constraints' and hence shouldn't end up in the architecture file | 13:57 |
| kem_ | Any thoughts? | 13:58 |
| mithro | kem_: Well - there are two seperate things - design constraints and architecture/device constraints | 13:58 |
| mithro | kem_: Real world device have all types of weird constraints like you can't use X if you use Y | 13:59 |
| daveshah | There is also the issue that the locations in VPR may not be the same as the actual locations, as the user would expect and the vendor tools use - for example, because of the need for padding or because of only considering a region of interest in the 7-series at the moment | 13:59 |
| mithro | An example of why we would want attributes on pb_types is to describe the functionality we are currently hard coding into the name https://github.com/SymbiFlow/symbiflow-arch-defs#names | 14:00 |
| tpb | Title: GitHub - SymbiFlow/symbiflow-arch-defs: FOSS architecture definitions of FPGA hardware useful for doing PnR device generation. (at github.com) | 14:00 |
| kem_ | mithro: I agree, it's just not clear to me where the architecture/device constraints should go. I would tend to lean towards trying to model the architecture correctly, rather than adding a (potentially unbounded) number of special cases. | 14:01 |
| kem_ | Also, the co-ordinate transform between VPR and real device should be fairly straightforward to do externally to VPR shouldn't it? | 14:03 |
| mithro | kem_: It is when you have a 1-1 mapping between VPR tiles and device tiles -- but it can get really messy when you end up representing a tile in the real device as multiple tiles | 14:04 |
| daveshah | Yes - the ECP5 for example can have situations like this, where multiple tiles occupy the same grid location | 14:05 |
| daveshah | https://usercontent.irccloud-cdn.com/file/4yyJ3FXA/ECP5%20tiles | 14:06 |
| kem_ | Couldn't you just specify the 1x1 region in VPR's grid space to correspond to the smallest tile? | 14:07 |
| kem_ | You'll end up with lots of blanks, but then the mapping would still be 1:1? | 14:08 |
| *** sunxi_fan has quit IRC | 14:08 | |
| mithro | kem_: Potentially | 14:09 |
| daveshah | kem_: Not sure whether or not that would create problems with channels though? | 14:10 |
| kem_ | daveshah: I don't think it should be an issue if your loading the real-device RR graph | 14:12 |
| kem_ | daveshah: you're* | 14:12 |
| daveshah | Yes, I think that will be the best way forward for the foreseeable future | 14:12 |
| mithro | kem_: It can be really useful to run with a vpr generated virtual rr_graph which approximates the real graph | 14:13 |
| kem_ | mithro: Agreed | 14:14 |
| kem_ | mithro: And you would still be able to, it just might be a much coarser approximation | 14:14 |
| mithro | kem_: That is where things can get really messy with that approach | 14:14 |
| kem_ | mithro: Valid point | 14:15 |
| mithro | kem_: I'd also like to try and keep a much of the architecture information inside the arch.xml file so we don't run into issues with files getting out of sync (plus it makes composability easier) | 14:16 |
| mithro | kem_: So that assert seems to be occuring because the tile doesn't have any RECEIVER pins... | 14:16 |
| mithro | int sink_ptc = get_best_class(RECEIVER, device_ctx.grid[sink_x][sink_y].type); | 14:17 |
| kem_ | mithro: A work around is to run with --timing_analysis off | 14:17 |
| kem_ | mithro: The problem is in placement delay profiling | 14:18 |
| kem_ | mithro: I was looking at it yesterday, and I know how to fix it but it requires refactoring the delay profiling to be more robust | 14:19 |
| kem_ | mithro and daveshah: The issues you guys bring up are all definitely valid | 14:21 |
| mithro | kem_: Would a fix like this work -> https://github.com/mithro/vtr-verilog-to-routing/commit/c6692b04fa6ec1496a3bdde1911b54aa12a480a5 ? | 14:22 |
| tpb | Title: Allow nodes without driver/receiver pins. · mithro/vtr-verilog-to-routing@c6692b0 · GitHub (at github.com) | 14:22 |
| kem_ | mithro: Yes that's the first stage of the fix, but it turns out to break the delay look-up since some delta values may no longer be valid | 14:23 |
| kem_ | mithro: Which requires re-working how we profile the delays to be more robust. I actually have most of the code to do this (written for other purposes) | 14:23 |
| mithro | Warning 3: Unable to route between blocks at (1,1) and (1,1) to characterize delay (setting to inf) | 14:24 |
| mithro | That warning ends up with | 14:24 |
| mithro | /usr/local/google/home/tansell/work/catx/vtr-verilog-to-routing/vpr/src/timing/timing_util.cpp:588 calc_relaxed_criticality: Assertion 'crit >= 0. - CRITICALITY_ROUND_OFF_TOLERANCE' failed (Criticality should never be negative). | 14:24 |
| kem_ | Yep thats the issue | 14:24 |
| mithro | Is that what you are referring too? | 14:24 |
| mithro | ahh great - I've hit that issue a bunch of times before | 14:24 |
| kem_ | It's not the most user friendly error message :( | 14:25 |
| kem_ | But fundamentally it means that the placement delay lookup got an inf delay (since the delay profiler failed to find a valid route between the profile locations choosen for that delta x and delta y) | 14:26 |
| kem_ | So the real fix is to make the profiling location selection more robust | 14:26 |
| mithro | okay | 14:26 |
| kem_ | Coming back to the grid/attributes issue | 14:27 |
| mithro | kem_: Back on the topic of attributes - I feel like it is useful to be to generally be able to tag things with arbitrary metadata | 14:27 |
| kem_ | mithro: I agree as well, in principle | 14:28 |
| kem_ | mithro: But what I want to avoid is adding a large number of special cases which handicap the flexibility of the code base | 14:29 |
| kem_ | mithro: I think there is a somewhat more fundamental issue here actually | 14:29 |
| kem_ | mithro: Something I've been kicking around in the back of my mind | 14:29 |
| mithro | kem_: A agree on principle of trying to reduce special cases :-) | 14:30 |
| kem_ | mithro: Since VPR's historic purpose has been architecture exploration, it's input specification (i.e. architecture file) is fundamentally under specified | 14:30 |
| kem_ | mithro: And VPR has a bunch of code to build up a more complete device description according to that specification | 14:31 |
| kem_ | mithro: Once that is done VPR's algorithms then map the design onto the complete device descritpion | 14:31 |
| mithro | Yeap | 14:32 |
| kem_ | mithro: For targeting real devices the interest is really in being able to specify that complete device description and using the impelementation algorithms | 14:32 |
| kem_ | mithro: The problem is that expanding the flexibility of the architecture format to allow that level of specification makes it less flexible | 14:33 |
| kem_ | mithro: That's actually somewhat of a tautological statement let me rephrase | 14:34 |
| mithro | kem_: Initially yes - I would however be very interested in being able to do things like take a real device and adding arbitrary extra restrictions on the top of it -- IE reducing the interconnect / routing available, etc | 14:34 |
| kem_ | mithro: This is just an idea, but it may make sense to have an intermediate format which describes the 'complete' device architure | 14:35 |
| kem_ | mithro: The rr_graph file is a partial implementation of this | 14:35 |
| kem_ | mithro: If we had this 'complete' device specification file, it could be generated to match a real device from some device DB, or built by VPR from a higher level architecture specification | 14:36 |
| kem_ | mithro: That would seem to be a logical place to put the types of attributes etc. that you're thinking of | 14:36 |
| kem_ | mithro: Anyway, just a thought... | 14:37 |
| mithro | kem_: I'm not necessarily convinced that allowing architecture exploration and real world devices are necessarily conflicting | 14:37 |
| mithro | kem_: For example, while building up a real world device description it is super useful to have vpr "fill out" the rest of the device with non-real stuff | 14:38 |
| mithro | kem_: I would also be very interested in having a vpr mode which lets vpr "find the right device for your design" | 14:39 |
| mithro | (IE which part size is your design likely to fit into) | 14:39 |
| kem_ | mithro: Those are all valid uses, and you may be right | 14:40 |
| kem_ | mithro: I think the 'find the right device' is still pretty straight-forward if you expand 'complete device specification file' to 'complete device family specification file' | 14:40 |
| mithro | kem_: Another thing I'm very interested in is being able to give direct feedback to FPGA manufacturers about how they should change their design to better fit real world needs - IE if you just add a /little/ bit more fabric of this type it would enable me to use a much smaller part for the same design | 14:41 |
| daveshah | It would also be able to have some kind of tiling ability, to minimise the amount of duplication and avoid having to have such a large file, for a device family specification file - unlike I believe how an rr_graph works at the moment for example | 14:42 |
| mithro | daveshah: Yeah - I actually think the arch.xml is pretty good at reducing duplication at the moment | 14:45 |
| kem_ | I agree exploiting tile-ability is definitely worth pursuing | 14:45 |
| mithro | kem_: So, it looks like I'm still getting rr_graph nodes with no edges :-/ | 14:45 |
| daveshah | mithro: Yes, that's exactly the sort of thing I'm thinking of. This goes back to the `sb_type` concept in #286 to some extent I feel | 14:45 |
| daveshah | But would possibly go further than that | 14:46 |
| mithro | daveshah / kem_: I actually think that the sb_type concept in #286 is general enough to describe both real world devices and to allow some very interesting academic exploration | 14:47 |
| mithro | daveshah / kem_: But I'm sure once we have an implementation it will be obvious there is a much better way to do it :-P | 14:48 |
| daveshah | It's also very similar to how I think Lattice Diamond is implemented internally, based on their physical viewer | 14:48 |
| daveshah | Which is a good sign :) | 14:48 |
| mithro | https://www.irccloud.com/pastebin/4m1kPUMA/ | 14:48 |
| tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 14:48 |
| kem_ | mithro: I guess that is part of my concern... it would be a lot of work to implement that type of specification and we'd want to be confident it was sufficiently general and not too difficult to use | 14:49 |
| kem_ | mithro: Having an intermediate complete device specification would mean it could be targeted my multiple potential 'front-end' architecture specifications without causing a lot of churn in VPR | 14:50 |
| kem_ | mithro: I'm not quite sure how to interpret that file format... | 14:51 |
| mithro | kem_: You mean the pastebin? | 14:51 |
| kem_ | mithro: Yea | 14:51 |
| kem_ | mithro: Is this coming from your RR graph generation tool? | 14:52 |
| mithro | X<grid x>Y<grid y>_<block name>[<ptc>].<rr_graph type> | 14:52 |
| mithro | kem_: Yes, I'm loading an rr_graph generated by vpr | 14:52 |
| daveshah | FYI - here is how the ECP5 routing and switchbox is drawn in Diamond | 14:53 |
| kem_ | mithro: Can you point me to the architecture? | 14:53 |
| daveshah | https://usercontent.irccloud-cdn.com/file/UlcGbTio/ecp5%20switchbox.png | 14:53 |
| daveshah | I think this is very close to how the `sb_type` might work | 14:54 |
| mithro | kem_: Same one I pointed in the previous bug -> https://gist.github.com/mithro/ef23fac984eb229a0300bbda22fca6b6#file-arch-xml | 14:54 |
| tpb | Title: Test Architecture for VPR · GitHub (at gist.github.com) | 14:54 |
| mithro | https://www.irccloud.com/pastebin/BlLWVyR8/ | 14:55 |
| tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 14:55 |
| mithro | kem_: That is the rr_graph.echo showing the same thing... | 14:55 |
| kem_ | mithro: There are some corner cases which an leave things disconnected... I'll take a look | 14:56 |
| mithro | I wouldn't assume my arch definition isn't borked in some way :-P | 14:56 |
| mithro | https://www.irccloud.com/pastebin/j3hYZbR0/ | 14:57 |
| tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 14:57 |
| mithro | I think the issue is these muxes -> https://gist.github.com/mithro/ef23fac984eb229a0300bbda22fca6b6#file-arch-xml-L132-L135 | 15:00 |
| tpb | Title: Test Architecture for VPR · GitHub (at gist.github.com) | 15:00 |
| mithro | It's definitely something to do with the inputs of the block | 15:05 |
| mithro | kem_: All SINK nodes should have at least one edge going into them, right? | 15:07 |
| kem_ | mithro: I'd assume so, but I want to figure out which sink nodes they are first | 15:07 |
| mithro | https://gist.github.com/mithro/ef23fac984eb229a0300bbda22fca6b6#file-arch-xml-L85 I believe | 15:09 |
| tpb | Title: Test Architecture for VPR · GitHub (at gist.github.com) | 15:09 |
| mithro | kem_: This looks like it is a bug in my code | 15:16 |
| mithro | kem_: I'm confused why the rr_graph.echo says zero edges though? | 15:16 |
| kem_ | mithro: Ah OK now I think I understand | 15:17 |
| kem_ | mithro: The RR graph is stored internally as a set of out-going edges associated with the source node | 15:18 |
| kem_ | mithro: So SINKs have no fanout, hence no edges | 15:18 |
| kem_ | mithro: The rr_graph.xml treats the edges as first-class elements listing both the source and sink | 15:18 |
| mithro | https://www.irccloud.com/pastebin/NJDhVAJl/ | 15:20 |
| tpb | Title: Snippet | IRCCloud (at www.irccloud.com) | 15:20 |
| mithro | kem_: So it seems like I just hit a bug in my code and a misunderstanding of the rr_graph.echo file | 15:23 |
| kem_ | mithro: Glad its figured out. I've been sticking to the rr_graph.xml mostly now in part because it is a bit more explicit than the .echo | 15:25 |
| kem_ | mithro: At some point we'll probably also drop the rr_graph.echo since it's superseded by the rr_graph.xml | 15:26 |
| mithro | kem_: Probably makes a lot of sense if you end up merging my rr_graph library as we can generate a bunch of human readable output from the rr_graph.xml file in Python | 15:26 |
| mithro | kem_: Gives you a lot more flexibility in how you output it too... | 15:27 |
| kem_ | mithro: Yep potentially | 15:28 |
| mithro | kem_: Welp, back to seeing if I can manipulate this rr_graph file :-P | 15:30 |
| daveshah | kem_: I got the buildbot failure email for my commits. I'm not certain, but I think this may be because the Titan benchmarks it fetches need to be updated with the renamed `<fc>` attributes? | 16:48 |
| *** digshadow has joined #vtr-dev | 16:57 | |
| mithro | kem_: Would you be opposed to the vpr GUI responding to the arrow keys? | 16:59 |
| kem_ | daveshah: Yes that makes sense. I'll take the action to fix that. | 17:00 |
| daveshah | kem_: Thanks! | 17:00 |
| daveshah | kem_: The arch file in GitHub is updated already, but I don't know if that's otherwise equivalent to the downloaded one? | 17:00 |
| mithro | kem_: One of my test FPGAs -> https://usercontent.irccloud-cdn.com/file/gknQOMRp/image.png | 17:01 |
| daveshah | mithro: BTW you can merge symbiflow-arch-defs #84 now | 17:01 |
| kem_ | daveshah: It's different because the Titan benchmarks are too big to keep directly in the VTR tree, so they have an independent copy of the architecture file | 17:02 |
| mithro | daveshah: I think I need to rebuilt vpr in conda first | 17:02 |
| kem_ | mithro: Responding to arrow keys seems fine, although note that you can also use middle-clock via the mouse to pan around | 17:03 |
| mithro | kem_: That seems to zoom for me | 17:04 |
| *** digshadow has quit IRC | 18:24 | |
| *** digshadow has joined #vtr-dev | 18:51 | |
| daveshah | I think a possible next step, certainly before actually doing anything in archfpga/VPR, for the new `sb_type` specification would be to try and write a Python script or similar to generate `sb_type`s for various real FPGAs (at minimum 7-series, iCE40 and ECP5), and that may give a better idea as to whether it is worth proceeding with | 19:24 |
| kem_ | mithro: Moving the mouse while holding middle-click should allow you to move around | 19:42 |
| kem_ | daveshah: Proving out sb_type externally seems like a good approach | 19:43 |
| *** digshadow has quit IRC | 21:16 | |
| kem_ | daveshah: I've updated the Titan release to fix the format changes, so BuildBot should now pass | 21:20 |
| *** digshadow has joined #vtr-dev | 21:29 | |
| mithro | kem_: Awesome thanks! | 21:30 |
| daveshah | kem_: I've started a rebuild to see what happens. | 21:32 |
| kem_ | daveshah: One odd thing about buildbot is a 'rebuild' re-tests the same changeset -- so it likely won't see the fix I commited | 21:39 |
| kem_ | daveshah: If we wait buildbot will see the new changeset and re-test | 21:40 |
| mithro | kem_: Wouldn't it be late for you? :-P | 21:50 |
| daveshah | Oops - indeed it failed again but the proper build with your changes has started now | 22:02 |
| daveshah | kem_: Updating titan has fixed it, it's run successfully now, thanks! | 22:40 |
Generated by irclog2html.py 2.13.1 by Marius Gedminas - find it at mg.pov.lt!
