> On Dec. 26, 2015, 11:27 p.m., Steve Reinhardt wrote:
> > src/mem/cache/cache.cc, line 2018
> > <http://reviews.gem5.org/r/3254/diff/2-3/?file=52272#file52272line2018>
> >
> >     again, this phrasing seems off; we're not passing the block as 
> > writable, and we're not relying on other caches to do things based on 
> > needsWritable... the other caches are simply obeying the obvious meaning of 
> > the isInvalidate flag, which means the requester ends up with a writable 
> > copy regardless of what we send it.

You are absolutely right. I'll make this more clear.


> On Dec. 26, 2015, 11:27 p.m., Steve Reinhardt wrote:
> > src/mem/packet.hh, line 270
> > <http://reviews.gem5.org/r/3254/diff/2-3/?file=52284#file52284line270>
> >
> >     I find the description from here on down confusing.  The owner is not 
> > passing the block as writable, the owner is just passing the block data 
> > along with ownership.  (How can it "pass" writability when it didn't have 
> > it to begin with?)  The writability comes from the fact that needsWritable 
> > -> isInvalidate, so all other copies are also invalidated as a side effect 
> > of the request.

Note that at the moment NeedsWritable is not always equal to IsInvalidate. They 
are two independent flags.

I agree with your statement though. I'll change it to say that we respond 
without removing writable, as we rely on all other copies being invalidated.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3254/#review7762
-----------------------------------------------------------


On Dec. 24, 2015, 8:59 a.m., Andreas Hansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3254/
> -----------------------------------------------------------
> 
> (Updated Dec. 24, 2015, 8:59 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11281:fc4962869323
> ---------------------------
> mem: Make cache terminology easier to understand
> 
> This patch changes the name of a bunch of packet flags and MSHR member
> functions and variables to make the coherency protocol easier to
> understand. In addition the patch adds and updates lots of
> descriptions, explicitly spelling out assumptions.
> 
> The following name changes are made:
> 
> * the packet memInhibit flag is renamed to cacheResponding
> 
> * the packet shared flag is renamed to passNonWritable
> 
> * the packet NeedsExclusive attribute is renamed to NeedsWritable
> 
> * the packet isSupplyExclusive is renamed responderHadWritable
> 
> * the MSHR pendingDirty is renamed to pendingWritable
> 
> The cache states, Modified, Owned, Exclusive, Shared are also called
> out in the cache and MSHR code to make it easier to understand.
> 
> 
> Diffs
> -----
> 
>   src/mem/cache/mshr.hh d9a0136ab8cc 
>   src/mem/cache/cache.cc d9a0136ab8cc 
>   src/mem/cache/cache.hh d9a0136ab8cc 
>   src/mem/cache/blk.hh d9a0136ab8cc 
>   src/mem/bridge.cc d9a0136ab8cc 
>   src/mem/cache/base.hh d9a0136ab8cc 
>   src/mem/addr_mapper.cc d9a0136ab8cc 
>   src/mem/dramsim2.cc d9a0136ab8cc 
>   src/mem/hmc_controller.cc d9a0136ab8cc 
>   src/mem/dram_ctrl.cc d9a0136ab8cc 
>   src/mem/comm_monitor.cc d9a0136ab8cc 
>   src/mem/coherent_xbar.cc d9a0136ab8cc 
>   src/mem/cache/mshr_queue.cc d9a0136ab8cc 
>   src/mem/cache/mshr.cc d9a0136ab8cc 
>   src/mem/cache/mshr_queue.hh d9a0136ab8cc 
>   src/mem/ruby/system/RubyPort.cc d9a0136ab8cc 
>   src/mem/ruby/system/DMASequencer.cc d9a0136ab8cc 
>   src/mem/packet.hh d9a0136ab8cc 
>   src/mem/packet.cc d9a0136ab8cc 
>   src/mem/noncoherent_xbar.cc d9a0136ab8cc 
>   src/mem/mem_checker_monitor.cc d9a0136ab8cc 
>   src/mem/snoop_filter.cc d9a0136ab8cc 
>   src/mem/tport.cc d9a0136ab8cc 
>   src/mem/simple_mem.cc d9a0136ab8cc 
>   src/mem/serial_link.cc d9a0136ab8cc 
>   src/mem/abstract_mem.cc d9a0136ab8cc 
>   src/cpu/o3/cpu.cc d9a0136ab8cc 
>   src/dev/dma_device.cc d9a0136ab8cc 
> 
> Diff: http://reviews.gem5.org/r/3254/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to