> On May 12, 2015, 3:55 p.m., Nilay Vaish wrote: > > You need to provide example of what this change is trying to achieve. > > Personally, if wanted buffers to have a size, I would want to that size > > to be configurable in python. > > Brad Beckmann wrote: > Would it be helpful if we described an example in the patch description? > This is obviously an improvement in functionality that people may want to use. > > I agree that eventually we would want to do this in Python. I'll let > Mark comment on that. > > Mark Wilkening wrote: > This change allows precise controll of specific individual buffer sizes > in a controller. Because these buffers are specific to a particular coherence > protocol (dependant on your gem5 build), it seems appropriate to set these > sizes in the SLICC file. This also does not have to be used, if not used the > default sizes for these buffers will be used instead. In the future we could > potentially set the sizes in python by setting sizes for buffers identified > by virtual network ID and network direction (in/out). This will require tight > coordination with the protocol definition to be used properly however. > > With this patch you can just set the size in SLICC like follows: > > MessageBuffer mandatoryQueue, ordered="false", buffer_size=32 > > MessageBuffer protocolSpecificQueue, network="out", virtual_network=4, > buffer_size=64
While I'm glad that we aim to add backpressure through Ruby, I agree with Nilay; This doesn't seem like the right way to set up buffer depth parameterization. We already expose the buffers in the Python code to connect them to the network. It would be easier and far more flexible to make MessageBuffers into SimObjects and allow the parameter to be set in Python. You wouldn't even need an init function (as Nilay suggests), but you could just pass the parameters to the C++ constructor during SimObject instantiation. Such an implementation would avoid changes to SLICC and this patch would need to be reverted. In addition to the usability issues, performance debugging bottlenecks resulting from buffer-sizing in the protocol files would likely be very difficult. There are the obvious issues with statically sizing buffers in the protocol files; the sizing will only make sense for particular system organizations but cause hard-to-expose bandwidth bottlenecks in others. Increasing the buffer size in the *.sm files and recompiling may fix this for your desired system, but to avoid recompiling for every different system organization, you'd end up with statically-defined, effectively infinite buffers again. Thus, the only *right* way to use this patch is to pass a parameter to the machine definition which is used for buffer sizing. Might as well pass the parameter directly to the message buffer in Python. - Joel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2814/#review6176 ----------------------------------------------------------- On May 11, 2015, 10:21 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2814/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 10:21 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10871:e121f9b1023e > --------------------------- > ruby: Changes to support finite buffering in ruby. > > Adds code in controller generating python code to initialize message buffers > with a size specified in the SLICC definition of of the controller's buffer. > This allows all message buffers including mandatory queues to use finite > buffering. > > Additionally adds code to properly check the resource constraints for an > action with respect to the entries available in a controller's network buffers > in the case that multiple buffers are defined to map to the same virtual > network. Previously this case would cause multiple resource checks to see if > a single entry was available, rather than one check to see if multiple entries > were available. > > > Diffs > ----- > > src/mem/slicc/ast/OutPortDeclAST.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/slicc/symbols/StateMachine.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2814/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
