----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3346/#review8082 -----------------------------------------------------------
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()? src/mem/cache/cache.cc (line 2260) <http://reviews.gem5.org/r/3346/#comment6971> e.g., just change this line to return sendMSHRQueuePacket(conflict_mshr); src/mem/cache/cache.cc (line 2266) <http://reviews.gem5.org/r/3346/#comment6972> similarly this would be return sendWriteQueuePacket(write_mshr); and so on for the remaining return statements... src/mem/cache/cache.cc (line 2322) <http://reviews.gem5.org/r/3346/#comment6973> I believe this would just be return false; src/mem/cache/cache.cc (line 2691) <http://reviews.gem5.org/r/3346/#comment6974> this would become waitingOnRetry = cache.sendNextPacket(); (or whatever else you may choose to rename getNextQueueEntry() once it incorporates the send*QueuePacket() methods) or maybe based on the discussion at the end above, if we need to pass in the queue pointer for the checkConflictingSnoop() call, it would be waitingonRetry = cache.sendNextPacket(this); src/mem/cache/cache.cc (line 2693) <http://reviews.gem5.org/r/3346/#comment6975> then this whole if/else if/else block goes away - Steve Reinhardt On March 13, 2016, 5:46 p.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3346/ > ----------------------------------------------------------- > > (Updated March 13, 2016, 5:46 p.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
