On Thu, Nov 4, 2010 at 9:41 AM, Ali Saidi <[email protected]> wrote: > > > > On 2010-11-03 20:08:42, Steve Reinhardt wrote: > > > Please split out the code changes and the stats changes into separate > changesets (as is our habit, if not yet our official policy)... the stats > changes are just too unwieldy and make it awkward to browse the code > changes. Thanks! > > I'll post the code changes separately, but when it comes to committing the > change I think we should commit code and stats changes together. The goal > should be the repository passes regressions at every revision. >
I saw your earlier mail and you have a good point but after some reflection I still don't agree. It sounds good to say that every revision should pass regressions, but in practice I don't know how much that matters. I certainly think that every revision should compile and run, because otherwise 'hg bisect' is a pain, but I've never used bisect to track down when a regression started failing... I guess it's possible, but if regressions are running regularly then we should catch failures soon enough that it's not such a mystery where things went wrong. Meanwhile it's nice to just be able to ignore the csets that are labeled as stats updates and not wade through them to find actual code changes. Perhaps even more compelling is that there are a number of times where people (including me) have made some pretty substantive changes that are spread out across several csets, and it's a big enough pain to regenerate stats that you really only want to do it once for a related sequence of changes, and once you cross that bridge then you're admitting that you're not going to have regressions pass at every rev. > > > > On 2010-11-03 20:08:42, Steve Reinhardt wrote: > > > src/cpu/simple/timing.cc, line 765 > > > <http://reviews.m5sim.org/r/256/diff/2/?file=4905#file4905line765> > > > > > > Does swapping the order of this OR really matter? > > Sorry, no... that is what I get for not paying enough attention to my diff. > I'll remove that part of it. > > > > On 2010-11-03 20:08:42, Steve Reinhardt wrote: > > > src/arch/alpha/isa/decoder.isa, line 791 > > > <http://reviews.m5sim.org/r/256/diff/2/?file=4902#file4902line791> > > > > > > Is there still a writeHint() method out there that just doesn't > ever get called now? Seems like if we're going to change the API to get rid > of writeHint() and prefetch() (which seems reasonable to me) we should make > sure we eliminate them thoroughly. > > prefetch() I'm happy with axing. If we wanted to do a write hint type > operation in the future, how would we do it? We could do a cache block sized > store, in which case I'm happy to axe it too... > I don't think writeHint() does anything anyway. If we want to add it back in we could just add a special flag to a write operation, which would be more consistent with other prefetches anyway. Steve > > > - Ali > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/256/#review431 > ----------------------------------------------------------- > > > On 2010-11-03 16:13:11, Ali Saidi wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://reviews.m5sim.org/r/256/ > > ----------------------------------------------------------- > > > > (Updated 2010-11-03 16:13:11) > > > > > > Review request for Default. > > > > > > Summary > > ------- > > > > ARM/Alpha/Cpu: Change prefetchs to be more like normal loads. > > > > This change modifies the way prefetches work. They are now like normal > loads > > that don't writeback a register. Previously prefetches were supposed to > call > > prefetch() on the exection context, so they executed with execute() > methods > > instead of initiateAcc() completeAcc(). The prefetch() methods for all > the CPUs > > are blank, meaning that they get executed, but don't actually do > anything. > > > > On Alpha dead cache copy code was removed and prefetches are now normal > ops. > > They count as executed operations, but still don't do anything and > IsMemRef is > > not longer set on them. > > > > On ARM IsDataPrefetch or IsInstructionPreftech is now set on all prefetch > > instructions. The timing simple CPU doesn't try to do anything special > for > > prefetches now and they execute with the normal memory code path. > > > > > > Diffs > > ----- > > > > src/arch/alpha/isa/decoder.isa 9d60c5339ae5 > > src/arch/alpha/isa/mem.isa 9d60c5339ae5 > > src/arch/arm/isa/insts/ldr.isa 9d60c5339ae5 > > src/cpu/simple/timing.cc 9d60c5339ae5 > > src/cpu/static_inst.hh 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-timing/config.ini > 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/00.gzip/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/config.ini > 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/simout > 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt > 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/config.ini > 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/simout 9d60c5339ae5 > > tests/long/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt > 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-atomic/config.ini 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-atomic/stats.txt 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-timing/config.ini 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/30.eon/ref/alpha/tru64/simple-timing/stats.txt 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-atomic/stats.txt > 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/config.ini > 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/40.perlbmk/ref/alpha/tru64/simple-timing/stats.txt > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/inorder-timing/config.ini > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/inorder-timing/simerr 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/inorder-timing/simout 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/inorder-timing/stats.txt > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-atomic/stats.txt > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-timing/config.ini > 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/50.vortex/ref/alpha/tru64/simple-timing/stats.txt > 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-atomic/stats.txt > 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-timing/config.ini > 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/60.bzip2/ref/alpha/tru64/simple-timing/stats.txt > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/inorder-timing/config.ini > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/inorder-timing/simerr 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/inorder-timing/simout 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/inorder-timing/stats.txt > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/o3-timing/config.ini 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/o3-timing/simout 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-atomic/simerr 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-atomic/stats.txt > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-timing/config.ini > 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-timing/simout 9d60c5339ae5 > > tests/long/70.twolf/ref/alpha/tru64/simple-timing/stats.txt > 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/linux/o3-timing/config.ini 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/linux/o3-timing/simout 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/tru64/o3-timing/stats.txt 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/tru64/simple-atomic/config.ini > 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/tru64/simple-atomic/simout 9d60c5339ae5 > > tests/quick/00.hello/ref/alpha/tru64/simple-atomic/stats.txt > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic-dual/config.ini > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic-dual/simout > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic-dual/stats.txt > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic/config.ini > 9d60c5339ae5 > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic/simout > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-atomic/stats.txt > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing-dual/config.ini > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing-dual/simout > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing-dual/stats.txt > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing/config.ini > 9d60c5339ae5 > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing/simout > 9d60c5339ae5 > > > tests/quick/10.linux-boot/ref/alpha/linux/tsunami-simple-timing/stats.txt > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-atomic/simerr > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-atomic/simout > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-atomic/stats.txt > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-timing/simerr > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-timing/simout > 9d60c5339ae5 > > tests/quick/20.eio-short/ref/alpha/eio/simple-timing/stats.txt > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-atomic-mp/simerr > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-atomic-mp/simout > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-atomic-mp/stats.txt > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-timing-mp/simerr > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-timing-mp/simout > 9d60c5339ae5 > > tests/quick/30.eio-mp/ref/alpha/eio/simple-timing-mp/stats.txt > 9d60c5339ae5 > > > tests/quick/80.netperf-stream/ref/alpha/linux/twosys-tsunami-simple-atomic/config.ini > 9d60c5339ae5 > > > tests/quick/80.netperf-stream/ref/alpha/linux/twosys-tsunami-simple-atomic/drivesys.terminal > 9d60c5339ae5 > > > tests/quick/80.netperf-stream/ref/alpha/linux/twosys-tsunami-simple-atomic/simout > 9d60c5339ae5 > > > tests/quick/80.netperf-stream/ref/alpha/linux/twosys-tsunami-simple-atomic/stats.txt > 9d60c5339ae5 > > > tests/quick/80.netperf-stream/ref/alpha/linux/twosys-tsunami-simple-atomic/testsys.terminal > 9d60c5339ae5 > > > > Diff: http://reviews.m5sim.org/r/256/diff > > > > > > Testing > > ------- > > > > > > Thanks, > > > > Ali > > > > > >
_______________________________________________ m5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/m5-dev
