----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1266/#review3028 -----------------------------------------------------------
Looks nice. Does it make sense to keep the old single-layer buses as an option? I agree they're not very realistic for modern designs, but they do model something that people have built in the past. I'm not sure if it's worth the hassle or not, but I thought I'd raise the question. src/mem/bus.hh <http://reviews.gem5.org/r/1266/#comment3232> Would you document this template parameter more? Obviously every bus connects MasterPorts to SlavePorts, so it's not immediately obvious what goes here. Also, I don't know if we have an official style rule for template parameter naming, but I know that all-caps is not common usage. I tend to make them camel case but with a suffix that indicates that it's not just a class name; e.g., in this case I would probably use PortClass. I'm happy to open a discussion on this if desired. src/mem/bus.hh <http://reviews.gem5.org/r/1266/#comment3233> I feel like we discussed "Port* port" vs "Port *port" before as well... do you recall what we ended up deciding? We should probably record that decision on the wiki. - Steve Reinhardt On June 21, 2012, 8:18 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1266/ > ----------------------------------------------------------- > > (Updated June 21, 2012, 8:18 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9079:5fb6386c07b4 > --------------------------- > Bus: Split the bus into separate request/response layers > > This patch splits the existing buses into multiple layers. The > non-coherent bus is split into a request and a response layer, and the > coherent bus adds an additional layer for the snoop responses. The > layer is modified to be templatised on the port type, such that the > different layers can have retryLists with either master or slave > ports. This patch also removes the dynamic cast from the retry, as > previously promised when moving the recvRetry from the port base class > to the master/slave port respectively. > > Overall, the split bus more closely reflects any modern on-chip bus > and should be at step in the right direction. From this point, it > would be reasonable straight forward to add separate layers (and thus > contention points and arbitration) for each port and thus create a > true crossbar. > > The regressions all produce the correct output, but have varying > degrees of changes to their statistics. A separate patch will be > pushed with the updates to the reference statistics. > > > Diffs > ----- > > src/mem/bus.hh d8e5ca139d7c > src/mem/bus.cc d8e5ca139d7c > src/mem/coherent_bus.hh d8e5ca139d7c > src/mem/coherent_bus.cc d8e5ca139d7c > src/mem/noncoherent_bus.hh d8e5ca139d7c > src/mem/noncoherent_bus.cc d8e5ca139d7c > > Diff: http://reviews.gem5.org/r/1266/diff/ > > > Testing > ------- > > util/regress all running and producing the right output (disregarding t1000 > and eio) but with essentially all timing tests exhibiting stat differences > reflecting less bus contention > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
