> On Feb. 29, 2016, 7:02 p.m., Jason Lowe-Power wrote: > > What is it about the GPU protocols that's different from all the other > > protocols? > > > > Also, does this mean that you can't use the DRAMCtrl with with GPU > > protocols anymore? I personally always use the DRAMCtrl instead of the Ruby > > memory controller. In fact, I think it's default for all other protocols. > > Matthew Poremba wrote: > Nothing particularly special, but the current queueMemory calls really > have no way to fail and simply push requests into the QueuedMasterPort. With > GPU applications I found it to be much easier to find cases that fill the > port's internal packet_queue and trigger an assertion if the size grows > larger than 100 packets. If these function calls are being removed, this > change will also need to be applied to the CPU protocols, but i'd like to get > a consensus between you, Joel, and others before spending time updating those. > > Jason Lowe-Power wrote: > I see. > > The problem I have with this patch (as I understand it) is that it seems > to undo changeset 10524:fff17530cef6 (ruby: interface with classic memory > controller). See http://reviews.gem5.org/r/2422/ > > I think it's a step in the wrong direction to go back to requiring the > Ruby memory controller. The classic DRAM model is much more flexible than > Ruby's (and more accurate as well, I believe). > > I could be off base here, though. Am I misunderstanding what this patch > does? > > Matthew Poremba wrote: > This change wouldn't go back to the Ruby memory controller. It is > dependent on another patch (http://reviews.gem5.org/r/3356/) which handles > consuming the MessageBuffers and sending them to the DRAMCtrl port. The > benefit here is to avoid infinite buffering that occurs when calling the > queueMemoryRead/Write functions. Since they return void, there is really no > way for the method to fail and provide back-pressure so they must push the > request to the port. This path leverages enqueue in SLICC, which is converted > to C++ code that first calls areNSlotsAvailable and allows Ruby to return a > ResourceStall if this returns false to prevent infinite buffering. I should > have put in the review that this patch also depends on > http://reviews.gem5.org/r/3356/ , particularly the changes in > AbstractController and StateMachine.py > > Joel has a similar solution in http://reviews.gem5.org/r/3332/ and > http://reviews.gem5.org/r/3331/. Our approach are slightly different since we > are trying to avoid using retries and NACKs as flow control.
Thanks for the clarification! I expected I was missing something. I'll take a look at http://reviews.gem5.org/r/3356/ soon and update this review as well. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3355/#review8050 ----------------------------------------------------------- On Feb. 29, 2016, 6:50 p.m., Matthew Poremba wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3355/ > ----------------------------------------------------------- > > (Updated Feb. 29, 2016, 6:50 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11355:02756e2f2053 > --------------------------- > ruby: Replace queueMemory calls with MessageBuffer enqueues in GPU protocols > > Calls to queueMemoryRead and queueMemoryWrite are removed in a previous > patch. This patch replaces these calls with enqueues to a MessageBuffer > which provides flow control to the directories. This modifies all GPU > protocols to use the new interface. > > > Diffs > ----- > > configs/ruby/GPU_RfO.py 31c5786945b447b372c3b7d346aea8fa6208577c > configs/ruby/GPU_VIPER.py 31c5786945b447b372c3b7d346aea8fa6208577c > configs/ruby/GPU_VIPER_Baseline.py 31c5786945b447b372c3b7d346aea8fa6208577c > configs/ruby/GPU_VIPER_Region.py 31c5786945b447b372c3b7d346aea8fa6208577c > src/mem/protocol/MOESI_AMD_Base-Region-dir.sm > 31c5786945b447b372c3b7d346aea8fa6208577c > src/mem/protocol/MOESI_AMD_Base-dir.sm > 31c5786945b447b372c3b7d346aea8fa6208577c > src/mem/protocol/MOESI_AMD_Base-probeFilter.sm > 31c5786945b447b372c3b7d346aea8fa6208577c > src/mem/protocol/RubySlicc_MemControl.sm > 31c5786945b447b372c3b7d346aea8fa6208577c > > Diff: http://reviews.gem5.org/r/3355/diff/ > > > Testing > ------- > > > Thanks, > > Matthew Poremba > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
