date:20161019

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

2016-10-19 Thread Anastasiia Butko



> On oct. 18, 2016, 1:55 après-midi, Jason Lowe-Power wrote:
> > configs/example/arm/fs_bigLITTLE.py, line 66
> > 
> >
> > Rather than having an extra parameter, could you make a subclass of the 
> > BigCluster? If this would introduce a lot more code, you shouldn't do it, 
> > but I think it would make things cleaner.
> 
> Anastasiia Butko wrote:
> Indeed, this will introduce more code. What about adding two new options, 
> --big-cpus-type and --little-cpus-type, while removing --ex5 and --atomic?
> 
> Jason Lowe-Power wrote:
> I'm a little surprised that it would introduce much more code. Overall, I 
> think it's *much* better to use object-oriented designs like inheritance 
> instead of passing extra parameters to the constructor.
> 
> You definitely shouldn't add more command line parameters! Just --ex5 is 
> fine. I like this script because it's short and sweet without loads of 
> command line options. IMO, if you want to model a system that has one ex5 
> core and one default core, you can modify the script or create a new script 
> to do that.

OK, I use one single option. But intead of creating multiple 
big_Cluater/little_Cluster classes I forward the desired type of core. It can 
be easily modified by user in case he wants a different configuration.


- Anastasiia


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


On oct. 20, 2016, 4:45 matin, Anastasiia Butko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3666/
> ---
> 
> (Updated oct. 20, 2016, 4:45 matin)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11687:b3946ea8b081
> ---
> arm, config: added support for ex5 model of big.LITTLE
> 
> This patch enables using calibrated big and LITTLE
> cores, ex5_big and ex5_LITTLE instead of the default
> 'arm_detailed' and 'minor' cpus. The ex5 model is based
> on the Samsung Exynos 5 Octa (5422) SoC. Operation and
> memory hierarchy latencies have been calibrated using the
> lmbench micro-benchmark suite. The preliminary validation
> results have been published as:
> 'Full-System Simulation of big.LITTLE Multicore Architecture
> for Performance and Energy Exploration', in International
> Symposium on Embedded Multicore/Many-core Systems-on-Chip
> (MCSoC'16), Lyon, France (Sep, 2016).
> 
> lat_ops bench results:
> --
>  bigLITTLE
> ops   SoC gem5SoC gem5
> --
> integer   bit:0.5 0.480.720.7
> integer   add:0.5 0.550.720.4
> integer   mul:1.561.682.161.87
> integer   div:45.36   36.32   53.347.32
> integer   mod:6.5618.55   17.37   18.02
> int64 bit:0.510.981.451.42
> uint64add:0.630.981.560.93
> int64 mul:2.522.133.644.68
> int64 div:92.61   195.52  213.14  197.63
> int64 mod:47.91   146.78  135.41  138.91
> float add:2.522.712.882.61
> float mul:3.022.842.882.73
> float div:9.078.9412.98   2.82
> doubleadd:2.522.712.882.61
> doublemul:3.022.845.042.73
> doublediv:16.13   10.43   23.03   2.81
> 
> lat_mem_rd bench results:
> --
>  bigLITTLE
> cache mem SoC gem5SoC gem5
> --
> l1/l1 0.5K4   4   3   3
>   1K  4   4   3   3
>   2K  4   4   3   3
>   3K  4   4   3   3
>   4K  4   4   3   3
>   6K  4   4   3   3
>   8K  4   4   3   3
>   12K 4   4   3   3
>   16K 4   18  3   3
>   24K 13  17  3   3
>   32K 18  18  6   11
> l2/l2 48K 23  20  9   14
>   64K 22  23  11  14
>   96K 23  23  13  14
>   128K23  23  14  15
>   192K23  23  15  17
>   256K23  23  17  17
>   384K23  24  29  17
>   512K23  24  59  39
> l2/Mem768K23  24  86  97
>   1M  23  24  112 110
>   1.5M35  25  153 147
>   2M  81  56  165 172
> Mem/Mem   3M  167 162

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

2016-10-19 Thread Anastasiia Butko


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

(Updated Oct. 20, 2016, 4:45 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11687:b3946ea8b081
---
arm, config: added support for ex5 model of big.LITTLE

This patch enables using calibrated big and LITTLE
cores, ex5_big and ex5_LITTLE instead of the default
'arm_detailed' and 'minor' cpus. The ex5 model is based
on the Samsung Exynos 5 Octa (5422) SoC. Operation and
memory hierarchy latencies have been calibrated using the
lmbench micro-benchmark suite. The preliminary validation
results have been published as:
'Full-System Simulation of big.LITTLE Multicore Architecture
for Performance and Energy Exploration', in International
Symposium on Embedded Multicore/Many-core Systems-on-Chip
(MCSoC'16), Lyon, France (Sep, 2016).

lat_ops bench results:
--
   bigLITTLE
ops SoC gem5SoC gem5
--
integer bit:0.5 0.480.720.7
integer add:0.5 0.550.720.4
integer mul:1.561.682.161.87
integer div:45.36   36.32   53.347.32
integer mod:6.5618.55   17.37   18.02
int64   bit:0.510.981.451.42
uint64  add:0.630.981.560.93
int64   mul:2.522.133.644.68
int64   div:92.61   195.52  213.14  197.63
int64   mod:47.91   146.78  135.41  138.91
float   add:2.522.712.882.61
float   mul:3.022.842.882.73
float   div:9.078.9412.98   2.82
double  add:2.522.712.882.61
double  mul:3.022.845.042.73
double  div:16.13   10.43   23.03   2.81

lat_mem_rd bench results:
--
   bigLITTLE
cache   mem SoC gem5SoC gem5
--
l1/l1   0.5K4   4   3   3
1K  4   4   3   3
2K  4   4   3   3
3K  4   4   3   3
4K  4   4   3   3
6K  4   4   3   3
8K  4   4   3   3
12K 4   4   3   3
16K 4   18  3   3
24K 13  17  3   3
32K 18  18  6   11
l2/l2   48K 23  20  9   14
64K 22  23  11  14
96K 23  23  13  14
128K23  23  14  15
192K23  23  15  17
256K23  23  17  17
384K23  24  29  17
512K23  24  59  39
l2/Mem  768K23  24  86  97
1M  23  24  112 110
1.5M35  25  153 147
2M  81  56  165 172
Mem/Mem 3M  167 162 170 179
4M  217 213 171 182
6M  251 247 171 182
8M  260 257 171 182
12M 263 261 171 182
16M 265 261 171 182

Reported-by: Anastasiia Butko 


Diffs (updated)
-

  configs/common/CpuConfig.py 4a86763c0b30 
  configs/common/ex5_LITTLE.py PRE-CREATION 
  configs/common/ex5_big.py PRE-CREATION 
  configs/example/arm/fs_bigLITTLE.py 4a86763c0b30 

Diff: http://reviews.gem5.org/r/3666/diff/


Testing
---


Thanks,

Anastasiia Butko

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

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

2016-10-19 Thread Anastasiia Butko


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

(Updated Oct. 20, 2016, 4:35 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11687:6293d0adc85c
---
arm, config: added support for ex5 model of big.LITTLE

This patch enables using calibrated big and LITTLE
cores, ex5_big and ex5_LITTLE instead of the default
'arm_detailed' and 'minor' cpus. The ex5 model is based
on the Samsung Exynos 5 Octa (5422) SoC. Operation and
memory hierarchy latencies have been calibrated using the
lmbench micro-benchmark suite. The preliminary validation
results have been published as:
'Full-System Simulation of big.LITTLE Multicore Architecture
for Performance and Energy Exploration', in International
Symposium on Embedded Multicore/Many-core Systems-on-Chip
(MCSoC'16), Lyon, France (Sep, 2016).

lat_ops bench results:
--
   bigLITTLE
ops SoC gem5SoC gem5
--
integer bit:0.5 0.480.720.7
integer add:0.5 0.550.720.4
integer mul:1.561.682.161.87
integer div:45.36   36.32   53.347.32
integer mod:6.5618.55   17.37   18.02
int64   bit:0.510.981.451.42
uint64  add:0.630.981.560.93
int64   mul:2.522.133.644.68
int64   div:92.61   195.52  213.14  197.63
int64   mod:47.91   146.78  135.41  138.91
float   add:2.522.712.882.61
float   mul:3.022.842.882.73
float   div:9.078.9412.98   2.82
double  add:2.522.712.882.61
double  mul:3.022.845.042.73
double  div:16.13   10.43   23.03   2.81

lat_mem_rd bench results:
--
   bigLITTLE
cache   mem SoC gem5SoC gem5
--
l1/l1   0.5K4   4   3   3
1K  4   4   3   3
2K  4   4   3   3
3K  4   4   3   3
4K  4   4   3   3
6K  4   4   3   3
8K  4   4   3   3
12K 4   4   3   3
16K 4   18  3   3
24K 13  17  3   3
32K 18  18  6   11
l2/l2   48K 23  20  9   14
64K 22  23  11  14
96K 23  23  13  14
128K23  23  14  15
192K23  23  15  17
256K23  23  17  17
384K23  24  29  17
512K23  24  59  39
l2/Mem  768K23  24  86  97
1M  23  24  112 110
1.5M35  25  153 147
2M  81  56  165 172
Mem/Mem 3M  167 162 170 179
4M  217 213 171 182
6M  251 247 171 182
8M  260 257 171 182
12M 263 261 171 182
16M 265 261 171 182

Reported-by: Anastasiia Butko 


Diffs (updated)
-

  configs/common/CpuConfig.py 4a86763c0b30 
  configs/common/ex5_LITTLE.py PRE-CREATION 
  configs/common/ex5_big.py PRE-CREATION 
  configs/example/arm/fs_bigLITTLE.py 4a86763c0b30 

Diff: http://reviews.gem5.org/r/3666/diff/


Testing
---


Thanks,

Anastasiia Butko

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

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

2016-10-19 Thread Anastasiia Butko


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

(Updated Oct. 20, 2016, 4:31 a.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11687:d06445524410
---
arm, config: added support for ex5 model of big.LITTLE

This patch enables using calibrated big and LITTLE
cores, ex5_big and ex5_LITTLE instead of the default
'arm_detailed' and 'minor' cpus. The ex5 model is based
on the Samsung Exynos 5 Octa (5422) SoC. Operation and
memory hierarchy latencies have been calibrated using the
lmbench micro-benchmark suite. The preliminary validation
results have been published as:
'Full-System Simulation of big.LITTLE Multicore Architecture
for Performance and Energy Exploration', in International
Symposium on Embedded Multicore/Many-core Systems-on-Chip
(MCSoC'16), Lyon, France (Sep, 2016).

lat_ops bench results:
--
   bigLITTLE
ops SoC gem5SoC gem5
--
integer bit:0.5 0.480.720.7
integer add:0.5 0.550.720.4
integer mul:1.561.682.161.87
integer div:45.36   36.32   53.347.32
integer mod:6.5618.55   17.37   18.02
int64   bit:0.510.981.451.42
uint64  add:0.630.981.560.93
int64   mul:2.522.133.644.68
int64   div:92.61   195.52  213.14  197.63
int64   mod:47.91   146.78  135.41  138.91
float   add:2.522.712.882.61
float   mul:3.022.842.882.73
float   div:9.078.9412.98   2.82
double  add:2.522.712.882.61
double  mul:3.022.845.042.73
double  div:16.13   10.43   23.03   2.81

lat_mem_rd bench results:
--
   bigLITTLE
cache   mem SoC gem5SoC gem5
--
l1/l1   0.5K4   4   3   3
1K  4   4   3   3
2K  4   4   3   3
3K  4   4   3   3
4K  4   4   3   3
6K  4   4   3   3
8K  4   4   3   3
12K 4   4   3   3
16K 4   18  3   3
24K 13  17  3   3
32K 18  18  6   11
l2/l2   48K 23  20  9   14
64K 22  23  11  14
96K 23  23  13  14
128K23  23  14  15
192K23  23  15  17
256K23  23  17  17
384K23  24  29  17
512K23  24  59  39
l2/Mem  768K23  24  86  97
1M  23  24  112 110
1.5M35  25  153 147
2M  81  56  165 172
Mem/Mem 3M  167 162 170 179
4M  217 213 171 182
6M  251 247 171 182
8M  260 257 171 182
12M 263 261 171 182
16M 265 261 171 182

Reported-by: Anastasiia Butko 


Diffs (updated)
-

  configs/common/CpuConfig.py 4a86763c0b30 
  configs/common/ex5_LITTLE.py PRE-CREATION 
  configs/common/ex5_big.py PRE-CREATION 
  configs/example/arm/fs_bigLITTLE.py 4a86763c0b30 

Diff: http://reviews.gem5.org/r/3666/diff/


Testing
---


Thanks,

Anastasiia Butko

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

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

2016-10-19 Thread Steve Reinhardt


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

Ship it!


Ship It!

- Steve Reinhardt


On Oct. 19, 2016, 3:30 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3662/
> ---
> 
> (Updated Oct. 19, 2016, 3:30 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11691:111bd8a24517
> ---
> syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc
> 
> The class was crammed into syscall_emul.hh which has tons of forward
> declarations and template definitions. To clean it up a bit, moved the
> class into separate files and commented the class with doxygen style
> comments. Also, provided some encapsulation by adding some accessors and
> a mutator.
> 
> The syscallreturn.hh file was renamed syscall_return.hh to make it consistent
> with other similarly named files in the src/sim directory.
> 
> The DPRINTF_SYSCALL macro was moved into its own header file with the
> include the Base and Verbose flags as well.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_debug_macros.hh PRE-CREATION 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/syscall_desc.cc PRE-CREATION 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3662/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

2016-10-19 Thread Steve Reinhardt


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

Ship it!


Yes, thanks!

- Steve Reinhardt


On Oct. 19, 2016, 12:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3671/
> ---
> 
> (Updated Oct. 19, 2016, 12:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11694:c915cc7cff42
> ---
> syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
> 
> The EIOProcess class was removed recently and it was the only other class
> which derived from Process. Since every Process invocation is also a
> LiveProcess invocation, it makes sense to simplify the who organization
> by combining the fields from LiveProcess into Process.
> 
> 
> Diffs
> -
> 
>   configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/simple.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/two_level.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/freebsd/freebsd.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/emul_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/syscall_desc.cc PRE-CREATION 
>

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

2016-10-19 Thread Brandon Potter


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

(Updated Oct. 19, 2016, 10:30 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11691:111bd8a24517
---
syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

The class was crammed into syscall_emul.hh which has tons of forward
declarations and template definitions. To clean it up a bit, moved the
class into separate files and commented the class with doxygen style
comments. Also, provided some encapsulation by adding some accessors and
a mutator.

The syscallreturn.hh file was renamed syscall_return.hh to make it consistent
with other similarly named files in the src/sim directory.

The DPRINTF_SYSCALL macro was moved into its own header file with the
include the Base and Verbose flags as well.


Diffs (updated)
-

  src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_debug_macros.hh PRE-CREATION 
  src/sim/syscall_desc.hh PRE-CREATION 
  src/sim/syscall_desc.cc PRE-CREATION 
  src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 

Diff: http://reviews.gem5.org/r/3662/diff/


Testing
---


Thanks,

Brandon Potter

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

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

2016-10-19 Thread Brandon Potter


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

(Updated Oct. 19, 2016, 10:25 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11691:ee7989e78d8b
---
syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

The class was crammed into syscall_emul.hh which has tons of forward
declarations and template definitions. To clean it up a bit, moved the
class into separate files and commented the class with doxygen style
comments. Also, provided some encapsulation by adding some accessors and
a mutator.

The syscallreturn.hh file was renamed syscall_return.hh to make it consistent
with other similarly named files in the src/sim directory.

The DPRINTF_SYSCALL macro was moved into its own header file with the
include the Base and Verbose flags as well.


Diffs (updated)
-

  src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_debug_macros.hh PRE-CREATION 
  src/sim/syscall_desc.hh PRE-CREATION 
  src/sim/syscall_desc.cc PRE-CREATION 
  src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 

Diff: http://reviews.gem5.org/r/3662/diff/


Testing
---


Thanks,

Brandon Potter

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

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

2016-10-19 Thread Jason Lowe-Power


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

Ship it!


Thanks for the change!

- Jason Lowe-Power


On Oct. 19, 2016, 7:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3671/
> ---
> 
> (Updated Oct. 19, 2016, 7:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11694:c915cc7cff42
> ---
> syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
> 
> The EIOProcess class was removed recently and it was the only other class
> which derived from Process. Since every Process invocation is also a
> LiveProcess invocation, it makes sense to simplify the who organization
> by combining the fields from LiveProcess into Process.
> 
> 
> Diffs
> -
> 
>   configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/simple.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/learning_gem5/part1/two_level.py 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/gpu-compute/cl_driver.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/freebsd/freebsd.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/emul_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/syscall_desc.cc PRE-CREATION 
>

Re: [gem5-dev] Review Request 3677: syscall_emul: [patch 11/22] extend functionality of fcntl

2016-10-19 Thread Steve Reinhardt


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


So do the flag values vary across ISAs/OSes? If not, there's no reason to make 
a template out of it (as there's no dependency on the ISA/OS).

- Steve Reinhardt


On Oct. 17, 2016, 9:05 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3677/
> ---
> 
> (Updated Oct. 17, 2016, 9:05 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11699:0d1798da97c9
> ---
> syscall_emul: [patch 11/22] extend functionality of fcntl
> 
> This changeset adds defines for new Linux flags for the fcntl system call,
> moves the fcntl implementation from syscall_emul.cc to syscall_emul.cc, and
> rewrites the system call.
> 
> 
> Diffs
> -
> 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3677/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class

2016-10-19 Thread Steve Reinhardt

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

src/sim/fd_array.hh (line 65)

I suggest calling this setFileOffsets() since it ends up calling 
setFileOffset() on each FDEntry. Or maybe updateFileOffsets(), if you think 
'set' is ambiguous. To me, 'find' doesn't imply a state update, so it's not as 
clear what this is doing.

src/sim/fd_array.hh (line 74)

Why doesn't a process that shares the same file descriptors just share the 
whole FDArray object?  This internal shared pointer thing is kinda awkward... 
which is OK if it's needed, but I don't understand why.

src/sim/fd_array.hh (line 140)

Can't these be static variables and not per-instance? I don't see where 
they're modified after they're constructed.

src/sim/fd_array.cc (line 262)

seems like this could be inlined in the header

src/sim/fd_array.cc (line 302)

this is a good candidate for inlining too

src/sim/process.cc (line 245)

if you're leaving this code in for it to be fixed later, it needs a comment 
to that effect, and would be cleaner if you put the whole loop inside "#if 0" 
rather than just commenting out the body.

src/sim/syscall_emul.hh (line 526)

I don't think this is true... the whole ENOTTY thing is for stdlib to 
figure out if stdout is buffered or not.

src/sim/syscall_emul.hh (line 995)

This seems unnecessarilly verbose. Unless you actually need fda somewhere 
else in the function, just do
process->fds[tgt_fd]
I'd say the same for fdp too; why define it if it's only going to be used 
once, unless (perhaps) there's a line length issue with the dynamic cast, but I 
hope that goes away as well

src/sim/syscall_emul.hh (line 997)

I agree with Michael, these dynamic casts are ugly. I think you should 
either:

A. Go all the way and define virtual functions on FDEntry to implement all 
the fd-related syscalls, so you can just replace the whole body of this 
function with
process->fds[tgt_fd]->fchmod(bufPtr)
(or something like that), and put what used to be the body of this function 
in FileFDEntry::fchmod(), or

B. Back off, put sim_fd back in FDentry, and just have the common syscalls 
like this one be oblivious to the FDEntry subclass. Presumably the host will do 
the right thing for pipes/sockets vs. files, and if you force sim_fd to -1 for 
emulated devices, you'll automatically get EBADF for those without putting in 
any additional checks.

- Steve Reinhardt

On Oct. 18, 2016, 1:01 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3676/
> ---
> 
> (Updated Oct. 18, 2016, 1:01 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11699:c22ac97e3372
> ---
> syscall_emul: [patch 10/22] refactor fdentry and add fdarray class
> 
> Several large changes happen in this patch.
> 
> The FDEntry class is rewritten so that file descriptors now correspond to
> types: 'Regular' which is normal file-backed file with the file open on the
> host machine, 'Pipe' which is a pipe that has been opened on the host machine,
> and 'Device' which does not have an open file on the host yet acts as a pseudo
> device with which to issue ioctls. Other types which might be added in the
> future are directory entries and sockets (off the top of my head).
> 
> The FDArray class was create to hold most of the file descriptor handling
> that was stuffed into the Process class. It uses shared pointers and
> the std::array type to hold the FDEntries mentioned above. The implementation
> could use a review; I feel that there's some room for improvement, but it
> seems like a decent first step.
> 
> The changes to these two classes needed to be propagated out to the rest
> of the code so there were quite a few changes for that. Also, comments were
> added where I thought they were needed to help others and extend our
> DOxygen coverage.
> 
> 
> Diffs
> -
> 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/fd_array.hh PRE-CREATION 
>   src/sim/fd_array.cc PRE-CREATION 
>   src/sim/fd_entry.hh

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

2016-10-19 Thread Brandon Potter



> On Oct. 17, 2016, 10:25 p.m., Jason Lowe-Power wrote:
> > src/sim/process.cc, line 593
> > 
> >
> > Any reason not to move this code into ProcessParams::create(), and 
> > eliminate this static function?
> 
> Brandon Potter wrote:
> Jason, I'm not sure what you want me to do here.
> 
> I put some gdb breaks on the create function to see how this is all 
> invoked. I get a backtrace where _wrap_ProcessParams::create 
> (build/X86/python/m5/internal/param_Process_wrap.cc\:5201) calls the static 
> function. It looks like Python is wrapped around the C++ create function and 
> is calling it.
> 
> The C++ method is static because the Process object (which ends up being 
> a derived type object who's type depends on the ISA) has not been created 
> yet; you can't call methods on an object that aren't statically declared. So, 
> the static function in the base class acts as a generator for the derived 
> Process class. Also, the ProcessParams must have already been initialized 
> because they're used within the Process::create function (so any new code 
> that was added would have to be put into the end of ProcessParams 
> initialization to make these variables available).
> 
> Regarding moving it into the Python code for params, it may be possible, 
> but I'm not sure how to do it. The macro "THE_ISA" is used to help figure out 
> what type of derived Process class to return and that's obvious in the code 
> how it's done in C++. I don't know of any examples for Python or how to go 
> about doing it.
> 
> Is there a particular reason that you want to remove the static function. 
> Have you run into problems with similar code in the past?
> 
> Jason Lowe-Power wrote:
> I certainly could be missing something here, but it seems like you could 
> take all of the code in `Process::create(ProcessParams * params)` and put it 
> in the body of `ProcessParams::create()`. You would need to replace the 
> `params` variable with `this`, but other than that, I don't see any changes. 
> Am I missing something?
> 
> It wasn't this that before, which I assumed was because LiveProcess was 
> one of multiple types of Process classes, but there could have been a good 
> reason. I haven't run into any problems with this code before. I was just 
> suggesting a little more housekeeping while you were here.
> 
> Brandon Potter wrote:
> Really, I'd rather not alter how this is currently set up. Have a hard 
> time seeing how I'd implement this and I don't believe that this will be a 
> simple copy-paste and make alterations job. It will probably involve 
> sleuthing around the Python code while straddling the SWIG boundary. I prefer 
> to stay away from that __black magic__ when possible.
> 
> Regarding housekeeping, I don't know why it's better to move this from 
> C++ into Python. The same code needs to exist in one form or another in 
> either C++ or Python.
> 
> If you feel strongly about it, I'd be happy to review an implementation 
> of it. However, I have no desire to actually go and do it myself.
> 
> Jason Lowe-Power wrote:
> This isn't a big deal, sorry to have belabored it. No worries if you 
> don't want to do it. I really just meant to move the code from lines 578 to 
> 720 and replace line 726. But don't worry about it! It's orthogonal to this 
> patch.
> 
> Steve Reinhardt wrote:
> Brandon, why do you think this would be difficult? I agree with Jason 
> that it should just be a matter of replacing
> Process::create(ProcessParams * params)
> with
> ProcessParams::create()
> and replacing 'params' with 'this' (actually replacing 'params->' with 
> ''), then getting rid of the current ProcessParams::create() definition.  All 
> we're talking about is collapsing a trivial function call on the C++ side, 
> not touching anything at all in Python.

I misunderstood what was being asked; apologies for being dense. Updated the 
code as requested.


- Brandon


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


On Oct. 19, 2016, 7:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3671/
> ---
> 
> (Updated Oct. 19, 2016, 7:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11694:c915cc7cff42
> ---
> syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead
> 
> The EIOProcess class was removed recently and it was the only other class
> which derived from Process. Since every Process invocation

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

2016-10-19 Thread Brandon Potter


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

(Updated Oct. 19, 2016, 7:27 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11694:c915cc7cff42
---
syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

The EIOProcess class was removed recently and it was the only other class
which derived from Process. Since every Process invocation is also a
LiveProcess invocation, it makes sense to simplify the who organization
by combining the fields from LiveProcess into Process.


Diffs (updated)
-

  configs/common/cpu2000.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/example/apu_se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/example/se.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/learning_gem5/part1/simple.py 
4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/learning_gem5/part1/two_level.py 
4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/splash2/cluster.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  configs/splash2/run.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/tru64/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/freebsd/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/solaris/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/linux/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/gpu-compute/cl_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/gpu-compute/cl_driver.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/freebsd/freebsd.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/operatingsystem.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/operatingsystem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/emul_driver.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_desc.hh PRE-CREATION 
  src/sim/syscall_desc.cc PRE-CREATION 
  src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
  src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 

Diff: http://reviews.gem5.org/r/3671/diff/


Testing
---


Thanks,

Brandon Potter

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

Re: [gem5-dev] Review Request 3674: syscall_emul: [patch 8/22] refactor process class

2016-10-19 Thread Steve Reinhardt


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



src/sim/aux_vector.hh (line 2)


In general, if you're creating a new file that contains pre-existing code 
from another file, the new file should inherit the original file's copyright 
and author list. Those apply to the code itself, and not the file as an entity. 
If you make changes or additions, you can then add the AMD 2016 copyright and 
your name to the author list. What you have here though makes it look like you 
wrote this code yourself from scratch in 2016, which is not right. I believe 
this applies in some of your other patches as well, though I neglected to flag 
it there.



src/sim/aux_vector.cc (line 42)


why do we need to include process.hh here?


- Steve Reinhardt


On Oct. 17, 2016, 8:23 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3674/
> ---
> 
> (Updated Oct. 17, 2016, 8:23 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11696:c48d378a972a
> ---
> syscall_emul: [patch 8/22] refactor process class
> 
> Moves aux_vector into its own .hh and .cc files just to get it out of the
> already crowded Process files. Arguably, it could stay there, but it's
> probably better just to move it and give it files.
> 
> The changeset looks ugly around the Process header file, but the goal here is
> to move methods and members around so that they're not defined randomly
> throughout the entire header file. I expect this is likely one of the reasons
> why I several unused variables related to this class. So, the methods are
> declared first followed by members. I've tried to aggregate them together
> so that similar entries reside near one another.
> 
> There are other changes coming to this code so this is by no means the
> final product.
> 
> 
> Diffs
> -
> 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/aux_vector.cc PRE-CREATION 
>   src/sim/aux_vector.hh PRE-CREATION 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3674/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3673: syscall_emul: [patch 7/22] remove numCpus method

2016-10-19 Thread Steve Reinhardt



> On Oct. 18, 2016, 8:42 a.m., Jason Lowe-Power wrote:
> > This is fine with me how it is. However, it's kind of moot if we're 
> > removing ALPHA support.
> 
> Brandon Potter wrote:
> Yeah, I am waiting to see if that materializes. If it does, I'll change 
> the patch to remove the numCpus method from Process.

Ha, I didn't look at this patch until after I sent my previous comment, but it 
was spookily prescient. As I indicated there, I'm opposed to removing Alpha 
entirely, but if we can clean up stuff like this by removing SE-mode support 
for Tru64, then I definitely support that.


- Steve


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


On Oct. 17, 2016, 8:20 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3673/
> ---
> 
> (Updated Oct. 17, 2016, 8:20 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11695:b71b3bd60a70
> ---
> syscall_emul: [patch 7/22] remove numCpus method
> 
> The numCpus method is misleading in that it's not really a measure of
> how many CPUs might be executing a process, but how many thread contexts
> are assigned to the process at any given point in time.
> 
> It's nice to highlight this distinction because thread contexts are never
> reused in the same way that a CPU can be reused for multiple processes.
> The reason that there is no reuse is that there is no CPU scheduler for SE.
> 
> The tru64 code intends to use this method and the accompanying contextIDs
> field to support SMT and track the number of threads with some system calls.
> With the up coming clone and exec patches, this paradigm must change. There
> needs to be a 1:1 mapping between the thread contexts and processes so that
> the process state between threads is allowed to vary when needed by Linux.
> This should not break SMT for tru64 if the Process class is refactored so that
> multiple Processes can share state between themselves. The following patches
> will do the refactoring incrementally as features are added.
> 
> 
> Diffs
> -
> 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3673/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3672: syscall_emul: [patch 6/22] remove unused fields from Process class

2016-10-19 Thread Steve Reinhardt

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

Ship it!

All that NXM support could probably go away at some point. We should also 
consider just eliminating SE-mode Tru64 support. I'm not sure anything uses it, 
and if so, it's probably stuff that people shouldn't be using anymore anyway.

Of course, I'm sure spme people named Andreas would say that that sentiment 
applies to Alpha support on the whole... but I think the overhead of 
maintaining Alpha Linux support is pretty low, at least comparable to some of 
the Tru64 stuff, in terms of wonky things that aren't hidden away in their own 
source files. Though I haven't looked at the code in a while so I could be 
wrong.

- Steve Reinhardt

On Oct. 17, 2016, 8:19 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3672/
> ---
> 
> (Updated Oct. 17, 2016, 8:19 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11694:ebff480ad584
> ---
> syscall_emul: [patch 6/22] remove unused fields from Process class
> 
> It looks like tru64 has some nxm* system calls, but the two fields that
> are defined in the Process class are unused by any of the code. There doesn't
> appear to be any reference in the tru64 code.
> 
> 
> Diffs
> -
> 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3672/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3670: syscall_emul: [patch 4/22] remove redundant M5_pid field from process

2016-10-19 Thread Steve Reinhardt

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

I agree that it's dumb to have two PIDs in the Process object, but until we 
figure out how we really want to fix the auto-assignment thing, it's probably 
premature to push this patch IMO. Let's wait until we have a full solution.

I'd be fine with doing something in python rather than C++; the way I see it, 
we already assign default values in python, so adding code to assign unique 
PIDs is really just a "smart default". That said, strictly speaking the Process 
class is not the best place for this, since really you'd want separate next-PID 
counters per System object. Making that happen automagically would be tricky 
though, and even the tricky solutions I can think of would be fragile in terms 
of perhaps swithcing around PID assignments based on unrelated config script 
changes. So off the top of my head, here's a proposal:

1. We continue to provide a fixed default PID in python to keep things simple 
for single-process runs.
2. For configs where the user is creating multiple Process objects per System, 
it's up to the user to manually assign unique PIDs. I think this is a good 
solution because it's an easy thing for users to do (just implement their own 
local counter in the python script), and yet is trivial from our side too. If 
we really wanted to, we could provide a get_next_pid() method on the Python 
System object, but I think that's overkill.
3. The System object needs to track all the PIDs of all the Process objects 
that are assigned to it at config time, and (1) call fatal() if the user builds 
a config with non-unique PIDs (with a helpful error message of course), and (2) 
do something to figure out how it can continue to assign unique PIDs when 
clone() is called. It might be possible to do #1 in python, but it's probably 
easier and more robust to do it in C++. Part 2 definitely needs to be on the 
C++ side, and could be as complicated as keeping a map of which PIDs are used, 
or as simple as just tracking the max manually assigned PID and then 
incrementing from there.

- Steve Reinhardt

On Oct. 17, 2016, 8:14 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3670/
> ---
> 
> (Updated Oct. 17, 2016, 8:14 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11692:ba60e4ae816f
> ---
> syscall_emul: [patch 4/22] remove redundant M5_pid field from process
> 
> 
> Diffs
> -
> 
>   src/sim/system.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/system.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/Process.py 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/faults.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3670/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3643: style: [patch 3/22] reduce include dependencies in some headers

2016-10-19 Thread Steve Reinhardt


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

Ship it!


Ship It!

- Steve Reinhardt


On Oct. 17, 2016, 8:12 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> ---
> 
> (Updated Oct. 17, 2016, 8:12 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11691:ba6ddbf68d8c
> ---
> style: [patch 3/22] reduce include dependencies in some headers
> 
> Used cppclean to help identify useless includes and removed them. This
> involved erroneously included headers, but also cases where forward
> declarations could have been used rather than a full include.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/alpha/tru64/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/interrupts.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/remote_gdb.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/isa_traits.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/pagetable.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/pseudo_inst.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/system.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/system.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/tlb.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/tlb.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/utility.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/utility.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/base/bitfield.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/base/bitunion.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/base/time.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/base/vnc/vncinput.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/minor/buffers.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/InvalidateGenerator.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/RubyDirectedTester.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/SeriesRequestGenerator.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/rubytest/Check.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/rubytest/CheckTable.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/flash_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/mc146818.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/net/dist_iface.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/net/etherbus.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/net/etherswitch.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/linux/linux.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/cache/prefetch/stride.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/external_master.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/external_slave.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/mem_checker.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/multi_level_page_table.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/multi_level_page_table_impl.hh 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/page_table.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/page_table.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/ruby/network/MessageBuffer.hh 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/ruby/structures/AbstractReplacementPolicy.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/se_translating_port_proxy.hh 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/mem/simple_mem.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/python/swig/pyevent.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>

Re: [gem5-dev] Review Request 3660: style: [patch 1/22] use /r/3648/ to reorganize includes

2016-10-19 Thread Steve Reinhardt


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

Ship it!


Ship It!

- Steve Reinhardt


On Oct. 17, 2016, 8:07 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3660/
> ---
> 
> (Updated Oct. 17, 2016, 8:07 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11689:579f5d74b7a6
> ---
> style: [patch 1/22] use /r/3648/ to reorganize includes
> 
> 
> Diffs
> -
> 
>   src/cpu/static_inst.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/DirectedGenerator.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/InvalidateGenerator.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/RubyDirectedTester.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/directedtest/SeriesRequestGenerator.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/garnet_synthetic_traffic/GarnetSyntheticTraffic.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/memtest/memtest.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/rubytest/Check.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/rubytest/CheckTable.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/rubytest/RubyTester.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/testers/traffic_gen/generators.cc 
> 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/thread_context.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/thread_state.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/cpu/timing_expr.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/alpha/backdoor.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/alpha/tsunami.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/alpha/tsunami_cchip.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/alpha/tsunami_io.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/a9scu.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/amba_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/amba_fake.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/energy_ctrl.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/gic_pl390.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/hdlcd.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/kmi.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/pl011.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/pl111.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/realview.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/rtc_pl031.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/timer_cpulocal.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/timer_sp804.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/arm/vgic.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/baddev.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/intel_8254_timer.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/io_device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/isa_fake.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/mc146818.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/mips/malta.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/mips/malta_cchip.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/mips/malta_io.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/pci/device.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/pci/host.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/platform.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/ps2.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/ps2.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/sparc/dtod.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/sparc/iob.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/sparc/mm_disk.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/sparc/t1000.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/uart.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/uart8250.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/virtio/base.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/virtio/block.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/virtio/console.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/virtio/fs9p.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/virtio/pci.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/dev/x86/cmos.cc

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

2016-10-19 Thread Steve Reinhardt


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



src/sim/syscall_desc.hh (line 67)


my preference would be to keep this as a one-liner as in the original code. 
I don't think expanding it makes it more readable. If anything, it makes the 
code seem more significant than it really is.



src/sim/syscall_desc.hh (line 73)


if you're going to create methods, might as well put the semantics here 
too. i.e., no need for separate warned() and setWarned() methods, just do 
something like:

bool
checkWarned()
{
bool was_warned = _warned;
_warned = true;
return was_warned;
}

and then the code in ignoreFunc() could be simplified a bit. I might even 
go further and, instead of checkWarned() like above, have a function like this:

bool
needWarning()
{
bool suppress_warning = warnOnce() && !_warned;
_warned = true;
return !suppress_warning;
}

and then you could rewrite ignoreFunc() along the lines of:

if (desc->needWarning()) {
warn("ignoring syscall %s(...)%s", desc->name(),
 desc->warnOnce() ? "\n  (further warnings will be 
suppressed)" : "");
}



src/sim/syscall_desc.hh (line 85)


again, this could be a one-liner.


- Steve Reinhardt


On Oct. 17, 2016, 8:08 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3662/
> ---
> 
> (Updated Oct. 17, 2016, 8:08 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11690:4bd82b7d3c09
> ---
> syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc
> 
> The class was crammed into syscall_emul.hh which has tons of forward
> declarations and template definitions. To clean it up a bit, moved the
> class into separate files and commented the class with doxygen style
> comments. Also, provided some encapsulation by adding some accessors and
> a mutator.
> 
> The syscallreturn.hh file was renamed syscall_return.hh to make it consistent
> with other similarly named files in the src/sim directory.
> 
> The DPRINTF_SYSCALL macro was moved into its own header file with the
> include the Base and Verbose flags as well.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_debug_macros.hh PRE-CREATION 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/syscall_desc.cc PRE-CREATION 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3662/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3675: syscall_emul: [patch 9/22] remove unused global variable (num_processes)

2016-10-19 Thread Steve Reinhardt


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

Ship it!


Ship It!

- Steve Reinhardt


On Oct. 17, 2016, 8:25 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3675/
> ---
> 
> (Updated Oct. 17, 2016, 8:25 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11697:e818bb00b9ea
> ---
> syscall_emul: [patch 9/22] remove unused global variable (num_processes)
> 
> 
> Diffs
> -
> 
>   src/arch/sparc/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

2016-10-19 Thread Steve Reinhardt



> On Oct. 11, 2016, 4:43 p.m., Tony Gutierrez wrote:
> > src/sim/syscall_desc.hh, line 66
> > 
> >
> > (void) is not necessary in c++. also this is not gem5 convention.
> 
> Brandon Potter wrote:
> I tried to get the same change past Steve once and he shot me down with 
> the same response. __Sigh__, I prefer my void being there (and it's 
> completely legal), but I'll change it for make benefit of glorious gem5.
> 
> Another common but unnecessary habit which I see quite a bit is using 
> "unsigned int" instead of just "unsigned". We'll have to crack down on those 
> folks now while we're at it. :)

It's legal, but I believe it's only there to provide backwards compatibility 
with C.  And the only reason this construct even exists in C is to distinguish 
prototypes of functions with no parameters with old-school K function 
declarations that don't provide parameter information (and which are not part 
of C++, and really should not be in any C code written in the last 25 years).  
So I think you should treat this feature as a historical anomaly and not as 
something you should be trying to exploit ;).


- Steve


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


On Oct. 17, 2016, 8:08 a.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3662/
> ---
> 
> (Updated Oct. 17, 2016, 8:08 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11690:4bd82b7d3c09
> ---
> syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc
> 
> The class was crammed into syscall_emul.hh which has tons of forward
> declarations and template definitions. To clean it up a bit, moved the
> class into separate files and commented the class with doxygen style
> comments. Also, provided some encapsulation by adding some accessors and
> a mutator.
> 
> The syscallreturn.hh file was renamed syscall_return.hh to make it consistent
> with other similarly named files in the src/sim directory.
> 
> The DPRINTF_SYSCALL macro was moved into its own header file with the
> include the Base and Verbose flags as well.
> 
> 
> Diffs
> -
> 
>   src/arch/alpha/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/freebsd/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/arm/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/mips/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/power/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/linux/syscalls.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/sparc/solaris/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/linux/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/arch/x86/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/kern/tru64/tru64.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/SConscript 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/process.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_debug_macros.hh PRE-CREATION 
>   src/sim/syscall_desc.hh PRE-CREATION 
>   src/sim/syscall_desc.cc PRE-CREATION 
>   src/sim/syscall_emul.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscall_emul.cc 4a86763c0b30cccba0f56c7f48637a46a4663b06 
>   src/sim/syscallreturn.hh 4a86763c0b30cccba0f56c7f48637a46a4663b06 
> 
> Diff: http://reviews.gem5.org/r/3662/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3622: Prefetch: Do not prefetch on cache & MSHR misses

2016-10-19 Thread Curtis Dunham


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


Interesting. This seems a problem worth correcting, but I fail to see how this 
change accomplishes it.  What prevents the notification from happening?  
Presumably there is some state being modified, but it is entirely unclear what 
or where.  Have you looked into that?

- Curtis Dunham


On Sept. 1, 2016, 12:10 p.m., Taehoon Kim wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3622/
> ---
> 
> (Updated Sept. 1, 2016, 12:10 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> This diff fixes incorrect prefetch notification.
> 
> If we set the option on_miss in BasePrefetch.py, prefetch should generated at 
> MSHR misses.
> However, prefetch controller doesn't generate prefetch request.
> 
> I just change the order of notify() in cache.cc
> 
> 
> Diffs
> -
> 
>   src/mem/cache/cache.cc a51ae096ca25 
> 
> Diff: http://reviews.gem5.org/r/3622/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Taehoon Kim
> 
>

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

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

2016-10-19 Thread Steve Reinhardt



> On Oct. 17, 2016, 3:25 p.m., Jason Lowe-Power wrote:
> > Seems reasonable to me, but do we have any historical context for 
> > "LiveProcess"?
> > 
> > Couple of small things below.
> 
> Brandon Potter wrote:
> I do not know about the historical context for LiveProcess. This is 
> speculation on my part, but I expect that the class just held new features 
> that were added that weren't needed for EIO. In the recent past (not sure 
> exactly when), Steve removed the EIOProcess. EIOProcess was supposed to allow 
> reading traces from SimpleScalar to help with bootstrapping SE mode for 
> Alpha. (At least, that's what I understood from a short converation that I 
> had with Steve.) As I mentioned in the changeset log, EIOProcess was the only 
> other class which derived from Process. So, I think that Process held 
> commonalities between EIO and Live, but it doesn't look like it was 
> maintained well. It seems like things were added without much thought for the 
> class organization.

I can confirm that the original (and only) reason for the split between Process 
and LiveProcess was for the former to factor out the things that were common 
between LiveProcess and EIOProcess. Since EIOProcess was rarely used and not 
part of the main code base, the distinction between Process and LiveProcess was 
not rigorously maintained as the two classes evolved. Now that EIOProcess is 
gone, there's no need to have two classes, and I think Brandon's patch is a 
useful simplification.


> On Oct. 17, 2016, 3:25 p.m., Jason Lowe-Power wrote:
> > src/sim/process.cc, line 593
> > 
> >
> > Any reason not to move this code into ProcessParams::create(), and 
> > eliminate this static function?
> 
> Brandon Potter wrote:
> Jason, I'm not sure what you want me to do here.
> 
> I put some gdb breaks on the create function to see how this is all 
> invoked. I get a backtrace where _wrap_ProcessParams::create 
> (build/X86/python/m5/internal/param_Process_wrap.cc\:5201) calls the static 
> function. It looks like Python is wrapped around the C++ create function and 
> is calling it.
> 
> The C++ method is static because the Process object (which ends up being 
> a derived type object who's type depends on the ISA) has not been created 
> yet; you can't call methods on an object that aren't statically declared. So, 
> the static function in the base class acts as a generator for the derived 
> Process class. Also, the ProcessParams must have already been initialized 
> because they're used within the Process::create function (so any new code 
> that was added would have to be put into the end of ProcessParams 
> initialization to make these variables available).
> 
> Regarding moving it into the Python code for params, it may be possible, 
> but I'm not sure how to do it. The macro "THE_ISA" is used to help figure out 
> what type of derived Process class to return and that's obvious in the code 
> how it's done in C++. I don't know of any examples for Python or how to go 
> about doing it.
> 
> Is there a particular reason that you want to remove the static function. 
> Have you run into problems with similar code in the past?
> 
> Jason Lowe-Power wrote:
> I certainly could be missing something here, but it seems like you could 
> take all of the code in `Process::create(ProcessParams * params)` and put it 
> in the body of `ProcessParams::create()`. You would need to replace the 
> `params` variable with `this`, but other than that, I don't see any changes. 
> Am I missing something?
> 
> It wasn't this that before, which I assumed was because LiveProcess was 
> one of multiple types of Process classes, but there could have been a good 
> reason. I haven't run into any problems with this code before. I was just 
> suggesting a little more housekeeping while you were here.
> 
> Brandon Potter wrote:
> Really, I'd rather not alter how this is currently set up. Have a hard 
> time seeing how I'd implement this and I don't believe that this will be a 
> simple copy-paste and make alterations job. It will probably involve 
> sleuthing around the Python code while straddling the SWIG boundary. I prefer 
> to stay away from that __black magic__ when possible.
> 
> Regarding housekeeping, I don't know why it's better to move this from 
> C++ into Python. The same code needs to exist in one form or another in 
> either C++ or Python.
> 
> If you feel strongly about it, I'd be happy to review an implementation 
> of it. However, I have no desire to actually go and do it myself.
> 
> Jason Lowe-Power wrote:
> This isn't a big deal, sorry to have belabored it. No worries if you 
> don't want to do it. I really just meant to move the code from lines 578 to 
> 720 and replace line 726. But don't worry about it! It's orthogonal to this 
> patch.

Brandon, why do you think this would be

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

2016-10-19 Thread Jason Lowe-Power



> On Oct. 18, 2016, 1:55 p.m., Jason Lowe-Power wrote:
> > configs/example/arm/fs_bigLITTLE.py, line 66
> > 
> >
> > Rather than having an extra parameter, could you make a subclass of the 
> > BigCluster? If this would introduce a lot more code, you shouldn't do it, 
> > but I think it would make things cleaner.
> 
> Anastasiia Butko wrote:
> Indeed, this will introduce more code. What about adding two new options, 
> --big-cpus-type and --little-cpus-type, while removing --ex5 and --atomic?

I'm a little surprised that it would introduce much more code. Overall, I think 
it's *much* better to use object-oriented designs like inheritance instead of 
passing extra parameters to the constructor.

You definitely shouldn't add more command line parameters! Just --ex5 is fine. 
I like this script because it's short and sweet without loads of command line 
options. IMO, if you want to model a system that has one ex5 core and one 
default core, you can modify the script or create a new script to do that.


- Jason


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


On Oct. 19, 2016, 4:22 a.m., Anastasiia Butko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3666/
> ---
> 
> (Updated Oct. 19, 2016, 4:22 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11687:0206e45662b9
> ---
> arm, config: added support for ex5 model of big.LITTLE
> 
> This patch enables using calibrated big and LITTLE
> cores, ex5_big and ex5_LITTLE instead of the default
> 'arm_detailed' and 'minor' cpus. The ex5 model is based
> on the Samsung Exynos 5 Octa (5422) SoC. Operation and
> memory hierarchy latencies have been calibrated using the
> lmbench micro-benchmark suite. The preliminary validation
> results have been published as:
> 'Full-System Simulation of big.LITTLE Multicore Architecture
> for Performance and Energy Exploration', in International
> Symposium on Embedded Multicore/Many-core Systems-on-Chip
> (MCSoC'16), Lyon, France (Sep, 2016).
> 
> lat_ops bench results:
> --
>  bigLITTLE
> ops   SoC gem5SoC gem5
> --
> integer   bit:0.5 0.480.720.7
> integer   add:0.5 0.550.720.4
> integer   mul:1.561.682.161.87
> integer   div:45.36   36.32   53.347.32
> integer   mod:6.5618.55   17.37   18.02
> int64 bit:0.510.981.451.42
> uint64add:0.630.981.560.93
> int64 mul:2.522.133.644.68
> int64 div:92.61   195.52  213.14  197.63
> int64 mod:47.91   146.78  135.41  138.91
> float add:2.522.712.882.61
> float mul:3.022.842.882.73
> float div:9.078.9412.98   2.82
> doubleadd:2.522.712.882.61
> doublemul:3.022.845.042.73
> doublediv:16.13   10.43   23.03   2.81
> 
> lat_mem_rd bench results:
> --
>  bigLITTLE
> cache mem SoC gem5SoC gem5
> --
> l1/l1 0.5K4   4   3   3
>   1K  4   4   3   3
>   2K  4   4   3   3
>   3K  4   4   3   3
>   4K  4   4   3   3
>   6K  4   4   3   3
>   8K  4   4   3   3
>   12K 4   4   3   3
>   16K 4   18  3   3
>   24K 13  17  3   3
>   32K 18  18  6   11
> l2/l2 48K 23  20  9   14
>   64K 22  23  11  14
>   96K 23  23  13  14
>   128K23  23  14  15
>   192K23  23  15  17
>   256K23  23  17  17
>   384K23  24  29  17
>   512K23  24  59  39
> l2/Mem768K23  24  86  97
>   1M  23  24  112 110
>   1.5M35  25  153 147
>   2M  81  56  165 172
> Mem/Mem   3M  167 162 170 179
>   4M  217 213 171 182
>   6M  251 247 171 182
>   8M  260 257 171 182
>   12M 263 261 171 182
>   16M 265 261 171 182
> 
> Reported-by: Anastasiia Butko 
>

[gem5-dev] changeset in gem5: stats: Update stats to reflect recent changes...

2016-10-19 Thread Andreas Hansson

changeset b3d5f0e9e258 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=b3d5f0e9e258
description:
stats: Update stats to reflect recent changes to floats

Mostly just splitting out the floats ops and corresponding
reads/writes.

diffstat:

 tests/long/fs/10.linux-boot/ref/alpha/linux/tsunami-minor/stats.txt
  |18 +-
 tests/long/fs/10.linux-boot/ref/alpha/linux/tsunami-o3-dual/stats.txt  
  |   198 +-
 tests/long/fs/10.linux-boot/ref/alpha/linux/tsunami-o3/stats.txt   
  |   104 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview-minor-dual/stats.txt
  |26 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview-minor/stats.txt 
  |18 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview-o3-dual/stats.txt   
  |98 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview-o3/stats.txt
  |46 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-minor-dual/stats.txt  
  |   104 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-minor/stats.txt   
  |32 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-o3-dual/stats.txt 
  |  6619 +
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-o3/stats.txt  
  |  2843 ++--
 
tests/long/fs/10.linux-boot/ref/arm/linux/realview64-simple-atomic-dual/stats.txt
|70 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-simple-atomic/stats.txt   
  |60 +-
 
tests/long/fs/10.linux-boot/ref/arm/linux/realview64-simple-timing-dual/stats.txt
|   104 +-
 tests/long/fs/10.linux-boot/ref/arm/linux/realview64-simple-timing/stats.txt   
  |60 +-
 tests/long/se/10.mcf/ref/arm/linux/minor-timing/stats.txt  
  |18 +-
 tests/long/se/10.mcf/ref/arm/linux/o3-timing/stats.txt 
  |40 +-
 tests/long/se/10.mcf/ref/sparc/linux/simple-timing/stats.txt   
  |18 +-
 tests/long/se/10.mcf/ref/x86/linux/o3-timing/stats.txt 
  |46 +-
 tests/long/se/10.mcf/ref/x86/linux/simple-timing/stats.txt 
  |18 +-
 tests/long/se/20.parser/ref/alpha/tru64/minor-timing/stats.txt 
  |18 +-
 tests/long/se/20.parser/ref/arm/linux/minor-timing/stats.txt   
  |16 +-
 tests/long/se/20.parser/ref/arm/linux/o3-timing/stats.txt  
  |38 +-
 tests/long/se/20.parser/ref/arm/linux/simple-atomic/stats.txt  
  |16 +-
 tests/long/se/20.parser/ref/arm/linux/simple-timing/stats.txt  
  |16 +-
 tests/long/se/20.parser/ref/x86/linux/o3-timing/stats.txt  
  |   100 +-
 tests/long/se/20.parser/ref/x86/linux/simple-atomic/stats.txt  
  |14 +-
 tests/long/se/20.parser/ref/x86/linux/simple-timing/stats.txt  
  |14 +-
 tests/long/se/30.eon/ref/alpha/tru64/minor-timing/stats.txt
  |18 +-
 tests/long/se/30.eon/ref/alpha/tru64/o3-timing/stats.txt   
  |   104 +-
 tests/long/se/30.eon/ref/alpha/tru64/simple-timing/stats.txt   
  |18 +-
 tests/long/se/30.eon/ref/arm/linux/minor-timing/stats.txt  
  |18 +-
 tests/long/se/30.eon/ref/arm/linux/o3-timing/stats.txt 
  |  1571 +-
 tests/long/se/30.eon/ref/arm/linux/simple-atomic/stats.txt 
  |18 +-
 tests/long/se/30.eon/ref/arm/linux/simple-timing/stats.txt 
  |18 +-
 tests/long/se/40.perlbmk/ref/alpha/tru64/minor-timing/stats.txt
  |18 +-
 tests/long/se/40.perlbmk/ref/alpha/tru64/o3-timing/stats.txt   
  |   104 +-
 tests/long/se/40.perlbmk/ref/alpha/tru64/simple-atomic/stats.txt   
  |18 +-
 tests/long/se/40.perlbmk/ref/alpha/tru64/simple-timing/stats.txt   
  |18 +-
 tests/long/se/40.perlbmk/ref/arm/linux/minor-timing/stats.txt  
  |18 +-
 tests/long/se/40.perlbmk/ref/arm/linux/o3-timing/stats.txt 
  |   104 +-
 tests/long/se/40.perlbmk/ref/arm/linux/simple-atomic/stats.txt 
  |18 +-
 tests/long/se/40.perlbmk/ref/arm/linux/simple-timing/stats.txt 
  |18 +-
 tests/long/se/50.vortex/ref/alpha/tru64/minor-timing/stats.txt 
  |18 +-
 tests/long/se/50.vortex/ref/alpha/tru64/o3-timing/stats.txt
  |46 +-
 tests/long/se/50.vortex/ref/arm/linux/minor-timing/stats.txt   
  |18 +-

Re: [gem5-dev] Review Request 3493: dev: Add 'simLength' parameter in EthPacketData

2016-10-19 Thread Andreas Sandberg


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

Ship it!


Ship It!

- Andreas Sandberg


On Oct. 12, 2016, 4:50 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3493/
> ---
> 
> (Updated Oct. 12, 2016, 4:50 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11637:cd6bb67002fa
> ---
> dev: Add 'simLength' parameter in EthPacketData
> Currently, all the network devices create a 16K buffer for the 'data' field
> in EthPacketData, and use 'length' to keep track of the size of the packet
> in the buffer.  This patch introduces the 'simLength' parameter to
> EthPacketData, which is used to hold the effective length of the packet used
> for all timing calulations in the simulator.  Serialization is performed using
> only the useful data in the packet ('length') and not necessarily the entire
> original buffer.
> 
> 
> Diffs
> -
> 
>   src/dev/net/i8254xGBe.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/ns_gige.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/pktfifo.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/etherbus.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/etherpkt.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/etherpkt.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/etherswitch.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/ethertap.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/sinic.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
> 
> Diff: http://reviews.gem5.org/r/3493/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

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

Re: [gem5-dev] Review Request 3665: arm, config: Enabled MemConfig usage for the example big.LITTLE

2016-10-19 Thread Andreas Sandberg


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

Ship it!


Ship It!

- Andreas Sandberg


On Oct. 19, 2016, 4:41 a.m., Anastasiia Butko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3665/
> ---
> 
> (Updated Oct. 19, 2016, 4:41 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11687:7b83be200f9e
> ---
> arm, config: Enabled MemConfig usage for the example big.LITTLE
> 
> This patch replaces the SimpleMemory with the MemConfig
> function. Additional parser arguments are added
> (TODO: improve with addCommonOptions & addFSOptions).
> 
> Reported-by: Anastasiia Butko 
> Reviewed-by: Andreas Sandberg 
> Reviewed-by: Jason Lowe-Power 
> 
> 
> Diffs
> -
> 
>   configs/example/arm/fs_bigLITTLE.py 4a86763c0b30 
> 
> Diff: http://reviews.gem5.org/r/3665/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anastasiia Butko
> 
>

___
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 quick

2016-10-19 Thread Cron Daemon

* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-atomic: 
CHANGED!* 
build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing: CHANGED!*** 
stat_diff: FAILURE: Statistics mismatchStatistics mismatch
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing: 
CHANGED!* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/minor-timing: 
CHANGED!*** diff[config.ini]: SKIPPED*** diff[simout]: SKIPPED
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing: CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/o3-timing: CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-atomic: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing-mt: 
CHANGED!* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-simple:
 CHANGED!
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-two-level:
 CHANGED!
* build/ALPHA/tests/opt/quick/se/30.eon/alpha/tru64/simple-atomic: CHANGED!
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-atomic: 
CHANGED!
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-timing: 
CHANGED!
*** diff[simout]: SKIPPED* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic: 
CHANGED!
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual:
 CHANGED!
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing: 
CHANGED!
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing-dual:
 CHANGED!
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-atomic: 
CHANGED!*** diff[smred.twf]: SKIPPED
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-timing: 
CHANGED!
* 
build/ALPHA/tests/opt/quick/fs/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic:
 CHANGED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer:
 CHANGED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer:
 CHANGED!
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MESI_Two_Level:
 CHANGED!*** gem5: OK
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MESI_Two_Level:
 CHANGED!
* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory:
 CHANGED!
* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory:
 CHANGED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token:
 CHANGED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token:
 CHANGED!
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-atomic: CHANGED!
*** gem5: OK* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/o3-timing: 
CHANGED!
*** diff[simout]: SKIPPED* 
build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing-ruby: 
CHANGED!*** gem5: OK
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing: 
CHANGED!*** stat_diff: FAILURE: Statistics mismatch
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-simple:
 CHANGED!
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-two-level:
 CHANGED!
* build/POWER/tests/opt/quick/se/00.hello/power/linux/simple-atomic: 
CHANGED!
* build/POWER/tests/opt/quick/se/00.hello/power/linux/o3-timing: CHANGED!
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/o3-timing: CHANGED!
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/simple-atomic: 
CHANGED!* 
build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing-ruby: CHANGED!
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-atomic: 
CHANGED!*** diff[config.ini]: SKIPPED
* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-two-level:
 CHANGED!
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing: 
CHANGED!
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/o3-timing-mp:
 CHANGED!
* 
build/SPARC/tests/opt/quick/se/40.m5threads-test-atomic/sparc/linux/simple-timing-mp:
 CHANGED!
* 
build/SPARC/tests/opt/quick/se/03.learning-gem5/sparc/linux/learning-gem5-p1-simple:
 CHANGED!Maximum error magnitude: +0.00%
* build/SPARC/tests/opt/quick/se/02.insttest/sparc/linux/simple-timing: 
CHANGED!
*

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

Re: [gem5-dev] Review Request 3677: syscall_emul: [patch 11/22] extend functionality of fcntl

Re: [gem5-dev] Review Request 3676: syscall_emul: [patch 10/22] refactor fdentry and add fdarray class

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

Re: [gem5-dev] Review Request 3674: syscall_emul: [patch 8/22] refactor process class

Re: [gem5-dev] Review Request 3673: syscall_emul: [patch 7/22] remove numCpus method

Re: [gem5-dev] Review Request 3672: syscall_emul: [patch 6/22] remove unused fields from Process class

Re: [gem5-dev] Review Request 3670: syscall_emul: [patch 4/22] remove redundant M5_pid field from process

Re: [gem5-dev] Review Request 3643: style: [patch 3/22] reduce include dependencies in some headers

Re: [gem5-dev] Review Request 3660: style: [patch 1/22] use /r/3648/ to reorganize includes

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

Re: [gem5-dev] Review Request 3675: syscall_emul: [patch 9/22] remove unused global variable (num_processes)

Re: [gem5-dev] Review Request 3662: syscall_emul: [patch 2/22] move SyscallDesc into its own .hh and .cc

Re: [gem5-dev] Review Request 3622: Prefetch: Do not prefetch on cache & MSHR misses

Re: [gem5-dev] Review Request 3671: syscall_emul: [patch 5/22] remove LiveProcess class and use Process instead

Re: [gem5-dev] Review Request 3666: arm, config: added support for ex5 model of big.LITTLE

[gem5-dev] changeset in gem5: stats: Update stats to reflect recent changes...

Re: [gem5-dev] Review Request 3493: dev: Add 'simLength' parameter in EthPacketData

Re: [gem5-dev] Review Request 3665: arm, config: Enabled MemConfig usage for the example big.LITTLE

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

29 matches

Site Navigation

Mail list logo

Footer information