----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1901/#review4418 -----------------------------------------------------------
Ship it! Looks great, I'm very glad to see all this code factored out. Just a couple of minor things: - When you change a file, please just add the ARM copyright without replacing the existing one, even if 99% of the file has been replaced. Nothing personal, but given how ARM gets added to the copyright list even for the most minor changes, it seems asymmetric to remove others' copyrights when the file has not been 100% replaced with something completely different. - I don't understand the logic of splitting this patch from the following one (1902), since you're already modifying some of the files under test/configs here. I think it's less confusing to have all the related changes in a single patch, but even if I were going to split it in two, I don't think I would choose this split. - Steve Reinhardt On June 11, 2013, 4:10 a.m., Andreas Hansson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1901/ > ----------------------------------------------------------- > > (Updated June 11, 2013, 4:10 a.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9772:b3272cd98ff7 > --------------------------- > config: Add a BaseSESystem builder for re-use in regressions > > This patch extends the existing system builders to also include a > syscall-emulation base builder. To start with, this builder is only > deployed in a limited number of cases, most notably o3-timing, > simple-timing and simple-atomic. The main reason for this is to not > change any regressions. > > At this point only a uni-processor builder is added, and the values > chosen for the cache sizes match those that were used in the existing > config scripts (despite being on the large side). > > A mem_class parameter is added to the builder base class to enable > simple-atomic to use SimpleMemory and o3-timing to use the default > DDR3 configuration. > > > Diffs > ----- > > tests/long/se/50.vortex/test.py 9df73385c878 > tests/long/se/60.bzip2/test.py 9df73385c878 > tests/long/se/70.twolf/test.py 9df73385c878 > tests/quick/se/00.hello/test.py 9df73385c878 > tests/quick/se/01.hello-2T-smt/test.py 9df73385c878 > tests/quick/se/02.insttest/test.py 9df73385c878 > tests/quick/se/20.eio-short/test.py 9df73385c878 > tests/long/se/10.mcf/test.py 9df73385c878 > tests/long/se/20.parser/test.py 9df73385c878 > tests/long/se/30.eon/test.py 9df73385c878 > tests/long/se/40.perlbmk/test.py 9df73385c878 > tests/configs/simple-timing.py 9df73385c878 > tests/configs/simple-atomic.py 9df73385c878 > tests/configs/simple-atomic-dummychecker.py 9df73385c878 > tests/configs/o3-timing.py 9df73385c878 > tests/configs/o3-timing-checker.py 9df73385c878 > tests/configs/inorder-timing.py 9df73385c878 > tests/configs/base_config.py 9df73385c878 > > Diff: http://reviews.gem5.org/r/1901/diff/ > > > Testing > ------- > > All regressions pass > > > Thanks, > > Andreas Hansson > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
