Re: [gem5-dev] Review Request 3525: ext: Update DRAMPower

2016-06-26 Thread Matthias Jung 


> On Juni 26, 2016, 9:59 nachm., Andreas Hansson wrote:
> > I realise I am commenting on DRAMPower changes here, so feel free to ignore.
> > 
> > There seems to be an awful lot of changes from unsigned to signed types, 
> > which feels rather unintuitive. Why not uint64_t if the value is truly 
> > unsigned? It seems odd to remove that clear communication of intent.
> > 
> > There also seems to be quite a few static_cast cause by the above 
> > change, which clutters the code. If the bank, for example, is unsigned this 
> > should not be needed.

there where some discussions about that issue in DRAMPowers git repro:

https://github.com/ravenrd/DRAMPower/issues/10#issuecomment-207067743


- Matthias


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


On Juni 22, 2016, 7:52 nachm., Matthias Jung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3525/
> ---
> 
> (Updated Juni 22, 2016, 7:52 nachm.)
> 
> 
> Review request for Default and Andreas Sandberg.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Sync DRAMPower to external tool
> 
> This patch syncs the DRAMPower library of gem5 to the external
> one on github (https://github.com/ravenrd/DRAMPower) of which
> I am a maintainer.
> 
> The version used is the commit:
> 902a00a1797c48a9df97ec88868f20e847680ae6
> from 07.  May.  2016.
> 
> 
> Diffs
> -
> 
>   ext/drampower/README.md dd6dfd38b6c2 
>   ext/drampower/src/CmdScheduler.h dd6dfd38b6c2 
>   ext/drampower/src/CmdScheduler.cc dd6dfd38b6c2 
>   ext/drampower/src/CommandAnalysis.h dd6dfd38b6c2 
>   ext/drampower/src/CommandAnalysis.cc dd6dfd38b6c2 
>   ext/drampower/src/MemArchitectureSpec.h dd6dfd38b6c2 
>   ext/drampower/src/MemCommand.h dd6dfd38b6c2 
>   ext/drampower/src/MemCommand.cc dd6dfd38b6c2 
>   ext/drampower/src/MemTimingSpec.h dd6dfd38b6c2 
>   ext/drampower/src/MemoryPowerModel.h dd6dfd38b6c2 
>   ext/drampower/src/MemoryPowerModel.cc dd6dfd38b6c2 
>   ext/drampower/src/MemorySpecification.h dd6dfd38b6c2 
>   ext/drampower/src/TraceParser.h dd6dfd38b6c2 
>   ext/drampower/src/TraceParser.cc dd6dfd38b6c2 
>   ext/drampower/src/Utils.h dd6dfd38b6c2 
>   ext/drampower/src/libdrampower/LibDRAMPower.h dd6dfd38b6c2 
>   ext/drampower/src/libdrampower/LibDRAMPower.cc dd6dfd38b6c2 
>   ext/drampower/test/libdrampowertest/lib_test.cc dd6dfd38b6c2 
>   src/mem/dram_ctrl.hh dd6dfd38b6c2 
> 
> Diff: http://reviews.gem5.org/r/3525/diff/
> 
> 
> Testing
> ---
> 
> Everything compiles.
> 
> 
> Thanks,
> 
> Matthias Jung
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Cron <m5test@zizzer> /z/m5/regression/do-regression --scratch all

2016-06-26 Thread Cron Daemon 
* 
build/SPARC/tests/opt/long/fs/80.solaris-boot/sparc/solaris/t1000-simple-atomic:
 CHANGED!
* build/ARM/tests/opt/long/fs/10.linux-boot/arm/linux/realview-o3-dual: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing-mt: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual:
 passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing: 
passed.
* build/ALPHA/tests/opt/quick/se/30.eon/alpha/tru64/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby: 
passed.
* build/ALPHA/tests/opt/quick/se/20.eio-short/alpha/eio/simple-atomic: 
passed.
* build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/simple-timing: passed.
* build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3: 
passed.
* build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-o3-dual: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing-dual:
 passed.
* build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/o3-timing: passed.
* build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/simple-atomic: 
passed.
* build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/simple-timing: 
passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-atomic: passed.
* build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/o3-timing: passed.
* build/ALPHA/tests/opt/long/se/30.eon/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby: 
passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/minor-timing: passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-two-level:
 passed.
* build/ALPHA/tests/opt/long/se/70.twolf/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby: 
passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-timing: passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-simple:
 passed.
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-timing: 
passed.
* build/ALPHA/tests/opt/long/se/60.bzip2/alpha/tru64/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-atomic: 
passed.
* build/ALPHA/tests/opt/long/se/60.bzip2/alpha/tru64/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing: passed.
* build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby: 
passed.
* 
build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-switcheroo-full:
 passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/30.eio-mp/alpha/eio/simple-timing-mp: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic:
 passed.
* build/ALPHA/tests/opt/quick/se/20.eio-short/alpha/eio/simple-timing: 
passed.
* build/ALPHA/tests/opt/long/se/50.vortex/alpha/tru64/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/30.eio-mp/alpha/eio/simple-atomic-mp: 
passed.
* build/ALPHA/tests/opt/long/fs/10.linux-boot/alpha/linux/tsunami-minor: 
passed.
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer:
 passed.
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer:
 passed.
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer:
 passed.
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer:
 passed.
* build/ALPHA/tests/opt/long/se/20.parser/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/long/se/40.perlbmk/alpha/tru64/o3-timing: passed.
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MESI_Two_Level:
 passed.
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MESI_Two_Level:
 passed.
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MESI_Two_Level:
 passed.
*

Re: [gem5-dev] Review Request 3525: ext: Update DRAMPower

2016-06-26 Thread Andreas Hansson 

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


I realise I am commenting on DRAMPower changes here, so feel free to ignore.

There seems to be an awful lot of changes from unsigned to signed types, which 
feels rather unintuitive. Why not uint64_t if the value is truly unsigned? It 
seems odd to remove that clear communication of intent.

There also seems to be quite a few static_cast cause by the above 
change, which clutters the code. If the bank, for example, is unsigned this 
should not be needed.


ext/drampower/src/CmdScheduler.cc (line 513)


this is rather unconventional

max(0, ...)

the construct appears in a few places



ext/drampower/src/CommandAnalysis.h (line 62)


really no point making the int parameter const



ext/drampower/src/MemArchitectureSpec.h (line 41)


cstdint rather?



ext/drampower/src/MemArchitectureSpec.h (line 51)


why are these signed?



ext/drampower/src/MemCommand.cc (line 95)


max(...


- Andreas Hansson


On June 22, 2016, 7:52 p.m., Matthias Jung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3525/
> ---
> 
> (Updated June 22, 2016, 7:52 p.m.)
> 
> 
> Review request for Default and Andreas Sandberg.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Sync DRAMPower to external tool
> 
> This patch syncs the DRAMPower library of gem5 to the external
> one on github (https://github.com/ravenrd/DRAMPower) of which
> I am a maintainer.
> 
> The version used is the commit:
> 902a00a1797c48a9df97ec88868f20e847680ae6
> from 07.  May.  2016.
> 
> 
> Diffs
> -
> 
>   ext/drampower/README.md dd6dfd38b6c2 
>   ext/drampower/src/CmdScheduler.h dd6dfd38b6c2 
>   ext/drampower/src/CmdScheduler.cc dd6dfd38b6c2 
>   ext/drampower/src/CommandAnalysis.h dd6dfd38b6c2 
>   ext/drampower/src/CommandAnalysis.cc dd6dfd38b6c2 
>   ext/drampower/src/MemArchitectureSpec.h dd6dfd38b6c2 
>   ext/drampower/src/MemCommand.h dd6dfd38b6c2 
>   ext/drampower/src/MemCommand.cc dd6dfd38b6c2 
>   ext/drampower/src/MemTimingSpec.h dd6dfd38b6c2 
>   ext/drampower/src/MemoryPowerModel.h dd6dfd38b6c2 
>   ext/drampower/src/MemoryPowerModel.cc dd6dfd38b6c2 
>   ext/drampower/src/MemorySpecification.h dd6dfd38b6c2 
>   ext/drampower/src/TraceParser.h dd6dfd38b6c2 
>   ext/drampower/src/TraceParser.cc dd6dfd38b6c2 
>   ext/drampower/src/Utils.h dd6dfd38b6c2 
>   ext/drampower/src/libdrampower/LibDRAMPower.h dd6dfd38b6c2 
>   ext/drampower/src/libdrampower/LibDRAMPower.cc dd6dfd38b6c2 
>   ext/drampower/test/libdrampowertest/lib_test.cc dd6dfd38b6c2 
>   src/mem/dram_ctrl.hh dd6dfd38b6c2 
> 
> Diff: http://reviews.gem5.org/r/3525/diff/
> 
> 
> Testing
> ---
> 
> Everything compiles.
> 
> 
> Thanks,
> 
> Matthias Jung
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev