> On March 14, 2016, 4:39 a.m., Steve Reinhardt wrote: > > Thanks for all the changes\\\! This is very close to what I was picturing, > > just one turn of the crank away (assuming you agree with the destination). > > All it would take would be to: > > > > \- Move the calls to checkConflictingSnoop() into > > sendMSHRQueuePacket()/sendWriteQueuePacket() > > \- Replace the returns in getNextQueueEntry() with calls to > > send*QueuePacket() > > > > Looking more closely I think I see why you didn't do this, which is that > > checkConflictingSnoop is a method on CacheReqPacketQueue which is maybe a > > pointer you don't have once you're in send*QueuePacket(). Can we get back > > to that structure via memSidePort, or if not, just pass it in as an > > argument through getNextQueueEntry() to send*QueuePacket()?
I do not think what you are proposing is achievable. The sendDeferredPacket method is called on the RequestPacketQueue, and it needs to update waitingOnRetry, and it also needs to be able to "suspend" schedulign if there is a conflict in the snoop response queue. Hence, the call to check for conflicts cannot be moved out of this method in any sensible fashion without returning tuples/pairs. Similarly, moving the call to the send function would complicate the call chain as the responsibility for the scheduling of events lies with the queue. The changes you propose may seem simple, but they completely muck with the subdivision of responsibilities. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3346/#review8082 ----------------------------------------------------------- On March 14, 2016, 12:46 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3346/ > ----------------------------------------------------------- > > (Updated March 14, 2016, 12:46 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11369:ebe8d4868008 > --------------------------- > mem: Create a separate class for the cache write buffer > > This patch breaks out the cache write buffer into a separate class, > without affecting any stats. The goal of the patch is to avoid > encumbering the much-simpler write queue with the complex MSHR > handling. In a follow on patch this simplification allows us to > implement write combining. > > The WriteQueue gets its own class, but shares a common ancestor, the > generic Queue, with the MSHRQueue. To distinguish between > WriteQueueEntries and MSHRs in the cache, a number of functions are > returning C++11 tuples as opposed to a single value. > > > Diffs > ----- > > src/mem/cache/SConscript 2fd64ea0a7cb > src/mem/cache/base.hh 2fd64ea0a7cb > src/mem/cache/base.cc 2fd64ea0a7cb > src/mem/cache/cache.hh 2fd64ea0a7cb > src/mem/cache/cache.cc 2fd64ea0a7cb > src/mem/cache/mshr.hh 2fd64ea0a7cb > src/mem/cache/mshr.cc 2fd64ea0a7cb > src/mem/cache/mshr_queue.hh 2fd64ea0a7cb > src/mem/cache/mshr_queue.cc 2fd64ea0a7cb > src/mem/cache/queue.hh PRE-CREATION > src/mem/cache/queue_entry.hh PRE-CREATION > src/mem/cache/write_queue.hh PRE-CREATION > src/mem/cache/write_queue.cc PRE-CREATION > src/mem/cache/write_queue_entry.hh PRE-CREATION > src/mem/cache/write_queue_entry.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3346/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
