> On March 23, 2015, 11:13 a.m., Steve Reinhardt wrote: > > src/mem/cache/cache_impl.hh, line 529 > > <http://reviews.gem5.org/r/2703/diff/1/?file=44266#file44266line529> > > > > how about just creating a block here (from before this decl to after > > the loop) to localize the scope of 'writebacks', so we don't have to bother > > with the assert at the bottom? > > Andreas Hansson wrote: > Could do if you think it makes things prettier. I think I'd rather create > a new variable with a different name if you think it is needed. > > Steve Reinhardt wrote: > I don't understand your response. 'writebacks' isn't used after the loop > (afaict), so closing the scope at that point would not just make it evident > that it's not used again, but enforce it at compile time. I'm not sure what > creating a new variable has to do with it, I assume because I'm just > misunderstanding you. > > Andreas Hansson wrote: > I could also just drop the assert since it is not used after the while > loop doing the writebacks. The assert is merely ensuring no one is adding any > new items after that. > > Andreas Hansson wrote: > Ah, now I understand what you are saying. Sure, I can add a "{" and "}" > around it. This would probably be the only place in the entire code base > where we use this trick though. > > Steve Reinhardt wrote: > "Narrow your scope with this one weird trick!" I agree we don't use > standalone blocks like this much, but I wouldn't consider it a trick :). I'm > sure there are several places where we have standalone blocks within the > cases of a switch, though you could argue that's just working around > deficiencies in the scoping of switches. > > Andreas Hansson wrote: > Hehe, sounds good. I've updated the patch. > > It would be good to get all these "fixes" for the cache done and dusted > before adding the locked RMW.
Agreed. I would like to push the more trivial patches (2688, 2689, 2690) ASAP to get them off my plate, but I'm happy to hold off on 2691 until these other things get in. - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2703/#review5963 ----------------------------------------------------------- On March 23, 2015, 11:54 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2703/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 11:54 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10765:751a74a1ff5b > --------------------------- > mem: Allocate cache writebacks before new MSHRs > > This patch changes the order of writeback allocation such that any > writebacks resulting from a tag lookup (e.g. for an uncacheable > access), are added to the writebuffer before any new MSHR entries are > allocated. This ensures that the writebacks logically precedes the new > allocations. > > The patch also changes the uncacheable flush to use proper timed (or > atomic) writebacks, as opposed to functional writes. > > > Diffs > ----- > > src/mem/cache/cache.hh 8a4040874157 > src/mem/cache/cache_impl.hh 8a4040874157 > > Diff: http://reviews.gem5.org/r/2703/diff/ > > > Testing > ------- > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
