> On July 2, 2012, 1:07 p.m., Andreas Hansson wrote: > > It looks reasonable. I am not entirely sure about the distinction of the > > packets though. What happens for shared I and D? Would it not be better to > > make it a property of the cache rather than the packet, i.e. a parameter on > > the class? > > Ali Saidi wrote: > I think adding a flag on the cache model is more complex and doesn't buy > much in practice. Dirty data can still live in a unified cache (because it > would get inserted by a write instead of an instruction fetch). The change > would need to be a property of the requesting cache, but it would need to > effect the sending cache in which case more fields would need to be added to > the packet. > > In summary, I still think that the way it's implemented is probably > preferable. > > Anthony Gutierrez wrote: > If it's a shared i/d cache, I would assume that a packet that is fetching > an instruction would still be distinguished as such. Also, I think a shared > i/d cache would cause several other issues beyond this. I.e., we couldn't > have the cache invalidation instructions, as they actually function in > ARM/x86, modeled in gem5 at all. E.g., we couldn't implement mcr icialluis as > it actually works, or if we did, we could never have dirty data in the L1. > > Andreas Hansson wrote: > All makes sense. Do we have to add some form of check to ensure that the > user-specified configuration adheres to specific rules? > > Anthony Gutierrez wrote: > I don't think so. I think any configuration, other than unifying I/D > caches, would work just fine with this. If somebody did want a unified I/D > cache they would have to make more modifications to gem5 other than configs. > > Ali Saidi wrote: > I'm pretty sure unified I/D will work too because while ownership wont > get transfered on an instruction read, it would get transfered on an data > write. Instruction cache flushing won't work, but at this point it doesn't > work now anyway. :) Are you happy with this change Andreas? > > Anthony Gutierrez wrote: > Right, any configuration should work with this. I was referring to a > situation where invalidation works and is implemented.
I'm happy, assuming the comments are updated to reflect the change :) - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1284/#review3011 ----------------------------------------------------------- On July 2, 2012, 11:21 a.m., Anthony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1284/ > ----------------------------------------------------------- > > (Updated July 2, 2012, 11:21 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9086:1c54688932a1 > --------------------------- > cache: don't allow dirty data in the i-cache > > removes the optimization that forwards an exclusive copy to a requester on a > read, only for the i-cache. this optimization isn't necessary because we > typically won't be writing to the i-cache. > > > Diffs > ----- > > src/mem/cache/cache_impl.hh 5f0321c03a2602f34dd03700e41e0cf5b47b8761 > > Diff: http://reviews.gem5.org/r/1284/diff/ > > > Testing > ------- > > Added i-cache invalidation instruction for ARM. These instructions invalidate > i-cache entries without writing them back, because it is not expected for > there to be dirty data in the caches. While invalidating I would get assert > failures and/or the program would hang due to dirty data in the i-cache being > invalidated. Doing this fixed those problems. Ran BBench to completion with > this fix and inv instructions in place. This is a necessary step towards > adding the inv instructions for ARM. > > > Thanks, > > Anthony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
