[gem5-dev] changeset in gem5: config: fix typo in cluster topology.

[Tushar Krishna]

changeset 220fa4099b9a in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=220fa4099b9a
description:
config: fix typo in cluster topology.

diffstat:

 configs/topologies/Cluster.py |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diffs (15 lines):

diff -r 380375085863 -r 220fa4099b9a configs/topologies/Cluster.py
--- a/configs/topologies/Cluster.py Fri Oct 07 14:14:44 2016 +0100
+++ b/configs/topologies/Cluster.py Fri Oct 07 23:56:48 2016 -0400
@@ -87,9 +87,9 @@
 
 # connect this cluster to the router
 link_out = IntLink(link_id=self.num_int_links(), 
src_node=self.router,
-   dst_node_=node.router)
+   dst_node=node.router)
 link_in = IntLink(link_id=self.num_int_links(), 
src_node=node.router,
-  dst_node_=self.router)
+  dst_node=self.router)
 
 if node.extBW:
 link_out.bandwidth_factor = node.extBW
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Krishna, Tushar]

Hi Brandon, Anthony,
Turned out to be a typo (dst_node was written as dst_node_) in the Cluster 
topology which is only used by APUs - hence I didn’t end up catching it when 
testing my patches earlier with other protocols.
I have fixed it and tested that it works. I have pushed the patch.

Cheers,
Tushar


> On Oct 7, 2016, at 7:26 PM, Gutierrez, Anthony  
> wrote:
> 
> Thanks, Tushar.
> 
> -Original Message-
> From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Krishna, Tushar
> Sent: Friday, October 07, 2016 4:01 PM
> To: gem5 Developer List 
> Subject: Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions
> 
> Thanks.
> I see the error too now. 
> It looks like the CPU-GPU system creates the topology in a somewhat different 
> manner than the pure CPU protocols that I am familiar with (and tested all my 
> changes with).
> Let me dig deeper into it and then send out a patch.
> 
> Thanks,
> Tushar
> 
> 
>> On Oct 7, 2016, at 6:37 PM, Gutierrez, Anthony  
>> wrote:
>> 
>> You can reproduce the error if you run apu_se.py by hand. For example:
>> 
>> # build the HSAIL GPU
>> scons -jN ./build/HSAIL_X86/gem5.opt
>> 
>> # run the GPU hello test
>> ./build/HSAIL_X86/gem5.opt configs/example/apu_se.py -c 
>> tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello -k 
>> tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello-kernel.asm
>> 
>> -Original Message-
>> From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of 
>> Krishna, Tushar
>> Sent: Friday, October 07, 2016 3:26 PM
>> To: gem5 Developer List 
>> Subject: Re: [gem5-dev] Review Request 3647: ruby: Fix broken 
>> regressions
>> 
>> Hi Brandon,
>> SimpleIntLink inherits from BasicIntLink (which defines dst_node) so this 
>> error shouldn’t ideally show up. I don’t see it when I run the simple 
>> network.
>> Can you tell me what command you ran?
>> I’ll try reproducing and debugging it on my end.
>> 
>> Cheers,
>> Tushar
>> 
>> 
>>> On Oct 7, 2016, at 6:15 PM, Andreas Hansson  wrote:
>>> 
>>> 
>>> 
 On Oct. 7, 2016, 10:04 p.m., Brandon Potter wrote:
> I'm noticing that this patch doesn't completely fix the regressions on 
> its own. There are still two failures that I'm seeing with it applied 
> (although they may be unrelated, not sure):
> 
> `cat
> build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_Rf
> O
> /simerr`
> 
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'dst_node_' already has parent 
> Traceback (most recent call last):
> File "", line 1, in   File 
> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
> line 400, in main
>  exec filecode in scope
> File 
> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py",
>  line 184, in 
>  execfile(joinpath(tests_root, 'configs', test_filename + '.py')) 
> File 
> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
>  line 282, in 
>  Ruby.create_system(options, None, system)  File 
> "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
> line 175, in create_system
>  Network.init_network(options, network, InterfaceClass)  File 
> "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
>  line 108, in init_network
>  network.setup_buffers()
> File 
> "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
>  line 62, in setup_buffers
>  if link.dst_node == router:
> File 
> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
>  line 1105, in __getattr__
>  raise AttributeError, err_string
> AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
> (C++ object is not yet constructed, so wrapped C++ methods are
> unavailable.)
> 
> `cat
> build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-
> r
> andomtest-ruby-GPU_RfO/simerr`
> 
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'int_node' already has parent
> warn: add_child('dst_node_'): child 'dst_node_' already has parent 
> Traceback (most recent call last):
> File "", line 1, in   File 
> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
> line 400, in main
>  exec filecode in scope
> File 
> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py",
>  line 184, in 
> 

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Tony Gutierrez]

> On Oct. 7, 2016, 7:15 a.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?
> 
> Andreas Hansson wrote:
> Excellent question. I think the topology line is acually both wrong and 
> unnecessary.
> 
> I really must confess the current behaviuor of addToPath is a bit of a 
> mystery (what is it really relative to), and it turns out it is relative to 
> the run.py script in the test folder.
> 
> Jason Lowe-Power wrote:
> Thanks for the info. I agree about addToPath :).
> 
> Tony Gutierrez wrote:
> You're right, according to the comment near the definition of addToPath() 
> it seems things are relative to whichever run script you're using. 
> Unfortunately the relative path of configs is not the same for all run 
> scripts. E.g., using apu_se.py by hand still doesn't work, even with this 
> patch, and I have to manually add the paths back to apu_se.py for things to 
> work.
> 
> Andreas Hansson wrote:
> Any suggestions for how to solve it?
> 
> Can this patch go in as is? If not, what should I add to it?
> 
> Tony Gutierrez wrote:
> I would suggest we make addToPath() behave more intuitively by having it 
> use the callers path, as opposed to whichever script initiated the python 
> interpreter - in our case the run scripts. This way the configs can truly use 
> paths relative to their own, which is not only more intuitive, but solves the 
> problem of the configs essentially needing to know the path relative to the 
> run scripts, which as previously pointed out may vary. The following worked 
> for apu_se.py paths:
> 
> diff --git a/configs/ruby/Ruby.py b/configs/ruby/Ruby.py
> --- a/configs/ruby/Ruby.py
> +++ b/configs/ruby/Ruby.py
> @@ -46,8 +46,8 @@
>  from m5.util import addToPath, fatal
>  
>  import MemConfig
> -addToPath('../configs/topologies')
> -addToPath('../configs/network')
> +addToPath('../topologies')
> +addToPath('../network')
>  
>  import Network
>  
> diff --git a/src/python/m5/util/__init__.py 
> b/src/python/m5/util/__init__.py
> --- a/src/python/m5/util/__init__.py
> +++ b/src/python/m5/util/__init__.py
> @@ -81,8 +81,10 @@
>  
>  # if it's a relative path and we know what directory the current
>  # python script is in, make the path relative to that directory.
> -if not os.path.isabs(path) and sys.path[0]:
> -path = os.path.join(sys.path[0], path)
> +frame = sys._current_frames().values()[0]
> +caller_path = os.path.dirname(frame.f_back.f_globals['__file__'])
> +if not os.path.isabs(path) and caller_path:
> +path = os.path.join(caller_path, path)
>  path = os.path.realpath(path)
>  # sys.path[0] should always refer to the current script's directory,
>  # so place the new dir right after that.
> 
> Then you'd need to go for all callers of addToPath() to specify paths 
> relative to their own, and not run.py.

Sorry, the diff in the above got messed up somehow. Should be:

diff --git a/configs/ruby/Ruby.py b/configs/ruby/Ruby.py
--- a/configs/ruby/Ruby.py
+++ b/configs/ruby/Ruby.py
@@ -46,8 +46,8 @@
 from m5.util import addToPath, fatal
 
 import MemConfig
-addToPath('../configs/topologies')
-addToPath('../configs/network')
+addToPath('../topologies')
+addToPath('../network')
 
 import Network
 
diff --git a/src/python/m5/util/__init__.py b/src/python/m5/util/__init__.py
--- a/src/python/m5/util/__init__.py
+++ b/src/python/m5/util/__init__.py
@@ -81,8 +81,10 @@
 
 # if it's a relative path and we know what directory the current
 # python script is in, make the path relative to that directory.
-if not os.path.isabs(path) and sys.path[0]:
-path = os.path.join(sys.path[0], path)
+frame = sys._current_frames().values()[0]
+caller_path = os.path.dirname(frame.f_back.f_globals['__file__'])
+if not os.path.isabs(path) and caller_path:
+path = os.path.join(caller_path, path)
 path = os.path.realpath(path)
 # sys.path[0] should always refer to the current script's directory,
 # so place the new dir right after that.


- Tony


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


On Oct. 7, 2016, 10:29 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 10:29 a.m.)
> 
> 
> Review 

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Tony Gutierrez]

> On Oct. 7, 2016, 7:15 a.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?
> 
> Andreas Hansson wrote:
> Excellent question. I think the topology line is acually both wrong and 
> unnecessary.
> 
> I really must confess the current behaviuor of addToPath is a bit of a 
> mystery (what is it really relative to), and it turns out it is relative to 
> the run.py script in the test folder.
> 
> Jason Lowe-Power wrote:
> Thanks for the info. I agree about addToPath :).
> 
> Tony Gutierrez wrote:
> You're right, according to the comment near the definition of addToPath() 
> it seems things are relative to whichever run script you're using. 
> Unfortunately the relative path of configs is not the same for all run 
> scripts. E.g., using apu_se.py by hand still doesn't work, even with this 
> patch, and I have to manually add the paths back to apu_se.py for things to 
> work.
> 
> Andreas Hansson wrote:
> Any suggestions for how to solve it?
> 
> Can this patch go in as is? If not, what should I add to it?

I would suggest we make addToPath() behave more intuitively by having it use 
the callers path, as opposed to whichever script initiated the python 
interpreter - in our case the run scripts. This way the configs can truly use 
paths relative to their own, which is not only more intuitive, but solves the 
problem of the configs essentially needing to know the path relative to the run 
scripts, which as previously pointed out may vary. The following worked for 
apu_se.py paths:

diff --git a/configs/ruby/Ruby.py b/configs/ruby/Ruby.py
--- a/configs/ruby/Ruby.py
+++ b/configs/ruby/Ruby.py
@@ -46,8 +46,8 @@
 from m5.util import addToPath, fatal
 
 import MemConfig
-addToPath('../configs/topologies')
-addToPath('../configs/network')
+addToPath('../topologies')
+addToPath('../network')
 
 import Network
 
diff --git a/src/python/m5/util/__init__.py b/src/python/m5/util/__init__.py
--- a/src/python/m5/util/__init__.py
+++ b/src/python/m5/util/__init__.py
@@ -81,8 +81,10 @@
 
 # if it's a relative path and we know what directory the current
 # python script is in, make the path relative to that directory.
-if not os.path.isabs(path) and sys.path[0]:
-path = os.path.join(sys.path[0], path)
+frame = sys._current_frames().values()[0]
+caller_path = os.path.dirname(frame.f_back.f_globals['__file__'])
+if not os.path.isabs(path) and caller_path:
+path = os.path.join(caller_path, path)
 path = os.path.realpath(path)
 # sys.path[0] should always refer to the current script's directory,
 # so place the new dir right after that.

Then you'd need to go for all callers of addToPath() to specify paths relative 
to their own, and not run.py.


- Tony


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


On Oct. 7, 2016, 10:29 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 10:29 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:8816ebf0a103
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated, and the messy path-deducing variations in
> the scripts are unified.
> 
> 
> Diffs
> -
> 
>   tests/configs/gpu-randomtest-ruby.py ebf2acd02fc5 
>   tests/configs/gpu-ruby.py ebf2acd02fc5 
>   tests/configs/memtest-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-ruby.py ebf2acd02fc5 
>   tests/configs/pc-simple-timing-ruby.py ebf2acd02fc5 
>   tests/configs/rubytest-ruby.py ebf2acd02fc5 
>   tests/configs/simple-atomic-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-ruby.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/ruby/Ruby.py ebf2acd02fc5 
> 
> Diff: 

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Gutierrez, Anthony]

Thanks, Tushar.

-Original Message-
From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Krishna, Tushar
Sent: Friday, October 07, 2016 4:01 PM
To: gem5 Developer List 
Subject: Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

Thanks.
I see the error too now. 
It looks like the CPU-GPU system creates the topology in a somewhat different 
manner than the pure CPU protocols that I am familiar with (and tested all my 
changes with).
Let me dig deeper into it and then send out a patch.

Thanks,
Tushar


> On Oct 7, 2016, at 6:37 PM, Gutierrez, Anthony  
> wrote:
> 
> You can reproduce the error if you run apu_se.py by hand. For example:
> 
> # build the HSAIL GPU
> scons -jN ./build/HSAIL_X86/gem5.opt
> 
> # run the GPU hello test
> ./build/HSAIL_X86/gem5.opt configs/example/apu_se.py -c 
> tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello -k 
> tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello-kernel.asm
> 
> -Original Message-
> From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of 
> Krishna, Tushar
> Sent: Friday, October 07, 2016 3:26 PM
> To: gem5 Developer List 
> Subject: Re: [gem5-dev] Review Request 3647: ruby: Fix broken 
> regressions
> 
> Hi Brandon,
> SimpleIntLink inherits from BasicIntLink (which defines dst_node) so this 
> error shouldn’t ideally show up. I don’t see it when I run the simple network.
> Can you tell me what command you ran?
> I’ll try reproducing and debugging it on my end.
> 
> Cheers,
> Tushar
> 
> 
>> On Oct 7, 2016, at 6:15 PM, Andreas Hansson  wrote:
>> 
>> 
>> 
>>> On Oct. 7, 2016, 10:04 p.m., Brandon Potter wrote:
 I'm noticing that this patch doesn't completely fix the regressions on its 
 own. There are still two failures that I'm seeing with it applied 
 (although they may be unrelated, not sure):
 
 `cat
 build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_Rf
 O
 /simerr`
 
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'dst_node_' already has parent 
 Traceback (most recent call last):
 File "", line 1, in   File 
 "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
 line 400, in main
   exec filecode in scope
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
 line 184, in 
   execfile(joinpath(tests_root, 'configs', test_filename + '.py')) 
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
  line 282, in 
   Ruby.create_system(options, None, system)  File 
 "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
 line 175, in create_system
   Network.init_network(options, network, InterfaceClass)  File 
 "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
  line 108, in init_network
   network.setup_buffers()
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
  line 62, in setup_buffers
   if link.dst_node == router:
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
  line 1105, in __getattr__
   raise AttributeError, err_string
 AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
 (C++ object is not yet constructed, so wrapped C++ methods are
 unavailable.)
 
 `cat
 build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-
 r
 andomtest-ruby-GPU_RfO/simerr`
 
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'int_node' already has parent
 warn: add_child('dst_node_'): child 'dst_node_' already has parent 
 Traceback (most recent call last):
 File "", line 1, in   File 
 "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
 line 400, in main
   exec filecode in scope
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
 line 184, in 
   execfile(joinpath(tests_root, 'configs', test_filename + '.py')) 
 File 
 "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-randomtest-ruby.py",
  line 104, in 
   Ruby.create_system(options, False, system)  File 
 "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
 line 175, in create_system
   Network.init_network(options, network, InterfaceClass)  File 
 "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
  

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Gutierrez, Anthony]

You can reproduce the error if you run apu_se.py by hand. For example:

# build the HSAIL GPU
scons -jN ./build/HSAIL_X86/gem5.opt

# run the GPU hello test
./build/HSAIL_X86/gem5.opt configs/example/apu_se.py -c 
tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello -k 
tests/test-progs/gpu-hello/bin/x86/linux/gpu-hello-kernel.asm

-Original Message-
From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Krishna, Tushar
Sent: Friday, October 07, 2016 3:26 PM
To: gem5 Developer List 
Subject: Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

Hi Brandon,
SimpleIntLink inherits from BasicIntLink (which defines dst_node) so this error 
shouldn’t ideally show up. I don’t see it when I run the simple network.
Can you tell me what command you ran?
I’ll try reproducing and debugging it on my end.

Cheers,
Tushar


> On Oct 7, 2016, at 6:15 PM, Andreas Hansson  wrote:
> 
> 
> 
>> On Oct. 7, 2016, 10:04 p.m., Brandon Potter wrote:
>>> I'm noticing that this patch doesn't completely fix the regressions on its 
>>> own. There are still two failures that I'm seeing with it applied (although 
>>> they may be unrelated, not sure):
>>> 
>>> `cat 
>>> build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_RfO
>>> /simerr`
>>> 
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'dst_node_' already has parent 
>>> Traceback (most recent call last):
>>>  File "", line 1, in   File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
>>> line 400, in main
>>>exec filecode in scope
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
>>> line 184, in 
>>>execfile(joinpath(tests_root, 'configs', test_filename + '.py'))  
>>> File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
>>>  line 282, in 
>>>Ruby.create_system(options, None, system)  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
>>> line 175, in create_system
>>>Network.init_network(options, network, InterfaceClass)  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
>>>  line 108, in init_network
>>>network.setup_buffers()
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
>>>  line 62, in setup_buffers
>>>if link.dst_node == router:
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
>>>  line 1105, in __getattr__
>>>raise AttributeError, err_string
>>> AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
>>>  (C++ object is not yet constructed, so wrapped C++ methods are 
>>> unavailable.)
>>> 
>>> `cat 
>>> build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-r
>>> andomtest-ruby-GPU_RfO/simerr`
>>> 
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'dst_node_' already has parent 
>>> Traceback (most recent call last):
>>>  File "", line 1, in   File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
>>> line 400, in main
>>>exec filecode in scope
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
>>> line 184, in 
>>>execfile(joinpath(tests_root, 'configs', test_filename + '.py'))  
>>> File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-randomtest-ruby.py",
>>>  line 104, in 
>>>Ruby.create_system(options, False, system)  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
>>> line 175, in create_system
>>>Network.init_network(options, network, InterfaceClass)  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
>>>  line 108, in init_network
>>>network.setup_buffers()
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
>>>  line 62, in setup_buffers
>>>if link.dst_node == router:
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
>>>  line 1105, in __getattr__
>>>raise AttributeError, err_string
>>> AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
>>>  (C++ object is not yet constructed, so wrapped C++ methods are 
>>> unavailable.)
> 
> I would think this is "unrelated" in that it has nothing to do with the 
> import of Network.
> 
> 
> - Andreas
> 
> 
> ---
> This is an automatically 

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Krishna, Tushar]

Hi Brandon,
SimpleIntLink inherits from BasicIntLink (which defines dst_node) so this error 
shouldn’t ideally show up. I don’t see it when I run the simple network.
Can you tell me what command you ran?
I’ll try reproducing and debugging it on my end.

Cheers,
Tushar


> On Oct 7, 2016, at 6:15 PM, Andreas Hansson  wrote:
> 
> 
> 
>> On Oct. 7, 2016, 10:04 p.m., Brandon Potter wrote:
>>> I'm noticing that this patch doesn't completely fix the regressions on its 
>>> own. There are still two failures that I'm seeing with it applied (although 
>>> they may be unrelated, not sure):
>>> 
>>> `cat 
>>> build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_RfO/simerr`
>>> 
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'dst_node_' already has parent
>>> Traceback (most recent call last):
>>>  File "", line 1, in 
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
>>> line 400, in main
>>>exec filecode in scope
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
>>> line 184, in 
>>>execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
>>>  line 282, in 
>>>Ruby.create_system(options, None, system)
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
>>> line 175, in create_system
>>>Network.init_network(options, network, InterfaceClass)
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
>>>  line 108, in init_network
>>>network.setup_buffers()
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
>>>  line 62, in setup_buffers
>>>if link.dst_node == router:
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
>>>  line 1105, in __getattr__
>>>raise AttributeError, err_string
>>> AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
>>>  (C++ object is not yet constructed, so wrapped C++ methods are 
>>> unavailable.)
>>> 
>>> `cat 
>>> build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-randomtest-ruby-GPU_RfO/simerr`
>>> 
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'int_node' already has parent
>>> warn: add_child('dst_node_'): child 'dst_node_' already has parent
>>> Traceback (most recent call last):
>>>  File "", line 1, in 
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
>>> line 400, in main
>>>exec filecode in scope
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
>>> line 184, in 
>>>execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-randomtest-ruby.py",
>>>  line 104, in 
>>>Ruby.create_system(options, False, system)
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
>>> line 175, in create_system
>>>Network.init_network(options, network, InterfaceClass)
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
>>>  line 108, in init_network
>>>network.setup_buffers()
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
>>>  line 62, in setup_buffers
>>>if link.dst_node == router:
>>>  File 
>>> "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
>>>  line 1105, in __getattr__
>>>raise AttributeError, err_string
>>> AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
>>>  (C++ object is not yet constructed, so wrapped C++ methods are 
>>> unavailable.)
> 
> I would think this is "unrelated" in that it has nothing to do with the 
> import of Network.
> 
> 
> - Andreas
> 
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/#review8790
> ---
> 
> 
> On Oct. 7, 2016, 5:29 p.m., Andreas Hansson wrote:
>> 
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> http://reviews.gem5.org/r/3647/
>> ---
>> 
>> (Updated Oct. 7, 2016, 5:29 p.m.)
>> 
>> 
>> Review request for Default.
>> 
>> 
>> Repository: gem5
>> 
>> 
>> Description
>> ---
>> 
>> Changeset 

Re: [gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Brandon Potter]

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


Do you think it's worthwhile to adopt a more detailed organization for headers?

Currently, we have this:
1) primary header (foo.hh for foo.cc)
2) mix of C/C++ headers alphabetically ordered
3) project headers alphabetically ordered

Should we move to something like the Google Style Guide's ruleset? 
https://google.github.io/styleguide/cppguide.html

It looks like this:
1) dir2/foo2.h.
2) C system files.
3) C++ system files.
4) Other libraries' .h files.
5) Your project's .h files.

Having the primary header on top is useful, but I do not know what the other 
ordering provides or that it protects us against anything. It does provide more 
organization so that it's easier to identify an include if you happen to glance 
up at the header sections, but that seems minor and probably isn't their 
justification. Does anyone else have thoughts on the topic?

- Brandon Potter


On Oct. 7, 2016, 3:11 p.m., Andreas Sandberg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3648/
> ---
> 
> (Updated Oct. 7, 2016, 3:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11669:258c76f88f24
> ---
> style: Add options to select checkers and apply fixes
> 
> Add an option, --checker/-c, to style.py that selects individual style
> checkers to apply. When this option isn't specified, the script
> defaults to all available style checkers. The option may be specified
> multiple times to run multiple style checkers.
> 
> The option, --fix/-f, can be specified to automatically fix style
> violations.
> 
> Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
> Signed-off-by: Andreas Sandberg 
> Reviewed-by: Andreas Hansson 
> 
> 
> Diffs
> -
> 
>   util/style.py 380375085863 
> 
> Diff: http://reviews.gem5.org/r/3648/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

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

Re: [gem5-dev] Review Request 3649: gpu-compute: add instruction mix stats for the gpu

[Jason Lowe-Power]

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

Ship it!


Ship It!

- Jason Lowe-Power


On Oct. 7, 2016, 9:03 p.m., Tony Gutierrez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3649/
> ---
> 
> (Updated Oct. 7, 2016, 9:03 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11675:4ce20a015e53
> ---
> gpu-compute: add instruction mix stats for the gpu
> 
> 
> Diffs
> -
> 
>   src/gpu-compute/compute_unit.hh 3803750858632ca6a80747881347846c0e44cba5 
>   src/gpu-compute/compute_unit.cc 3803750858632ca6a80747881347846c0e44cba5 
>   src/gpu-compute/wavefront.cc 3803750858632ca6a80747881347846c0e44cba5 
> 
> Diff: http://reviews.gem5.org/r/3649/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Andreas Hansson]

> On Oct. 7, 2016, 10:04 p.m., Brandon Potter wrote:
> > I'm noticing that this patch doesn't completely fix the regressions on its 
> > own. There are still two failures that I'm seeing with it applied (although 
> > they may be unrelated, not sure):
> > 
> > `cat 
> > build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_RfO/simerr`
> > 
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'dst_node_' already has parent
> > Traceback (most recent call last):
> >   File "", line 1, in 
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
> > line 400, in main
> > exec filecode in scope
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
> > line 184, in 
> > execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
> >  line 282, in 
> > Ruby.create_system(options, None, system)
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
> > line 175, in create_system
> > Network.init_network(options, network, InterfaceClass)
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
> >  line 108, in init_network
> > network.setup_buffers()
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
> >  line 62, in setup_buffers
> > if link.dst_node == router:
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
> >  line 1105, in __getattr__
> > raise AttributeError, err_string
> > AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
> >   (C++ object is not yet constructed, so wrapped C++ methods are 
> > unavailable.)
> > 
> > `cat 
> > build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-randomtest-ruby-GPU_RfO/simerr`
> > 
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'int_node' already has parent
> > warn: add_child('dst_node_'): child 'dst_node_' already has parent
> > Traceback (most recent call last):
> >   File "", line 1, in 
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
> > line 400, in main
> > exec filecode in scope
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", 
> > line 184, in 
> > execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-randomtest-ruby.py",
> >  line 104, in 
> > Ruby.create_system(options, False, system)
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
> > line 175, in create_system
> > Network.init_network(options, network, InterfaceClass)
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py",
> >  line 108, in init_network
> > network.setup_buffers()
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
> >  line 62, in setup_buffers
> > if link.dst_node == router:
> >   File 
> > "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py",
> >  line 1105, in __getattr__
> > raise AttributeError, err_string
> > AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
> >   (C++ object is not yet constructed, so wrapped C++ methods are 
> > unavailable.)

I would think this is "unrelated" in that it has nothing to do with the import 
of Network.


- Andreas


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


On Oct. 7, 2016, 5:29 p.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 5:29 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:8816ebf0a103
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated, and the messy path-deducing variations in
> the scripts are unified.
> 
> 
> Diffs
> -
> 
>   

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Andreas Hansson]

> On Oct. 7, 2016, 2:15 p.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?
> 
> Andreas Hansson wrote:
> Excellent question. I think the topology line is acually both wrong and 
> unnecessary.
> 
> I really must confess the current behaviuor of addToPath is a bit of a 
> mystery (what is it really relative to), and it turns out it is relative to 
> the run.py script in the test folder.
> 
> Jason Lowe-Power wrote:
> Thanks for the info. I agree about addToPath :).
> 
> Tony Gutierrez wrote:
> You're right, according to the comment near the definition of addToPath() 
> it seems things are relative to whichever run script you're using. 
> Unfortunately the relative path of configs is not the same for all run 
> scripts. E.g., using apu_se.py by hand still doesn't work, even with this 
> patch, and I have to manually add the paths back to apu_se.py for things to 
> work.

Any suggestions for how to solve it?

Can this patch go in as is? If not, what should I add to it?


- Andreas


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


On Oct. 7, 2016, 5:29 p.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 5:29 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:8816ebf0a103
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated, and the messy path-deducing variations in
> the scripts are unified.
> 
> 
> Diffs
> -
> 
>   tests/configs/gpu-randomtest-ruby.py ebf2acd02fc5 
>   tests/configs/gpu-ruby.py ebf2acd02fc5 
>   tests/configs/memtest-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-ruby.py ebf2acd02fc5 
>   tests/configs/pc-simple-timing-ruby.py ebf2acd02fc5 
>   tests/configs/rubytest-ruby.py ebf2acd02fc5 
>   tests/configs/simple-atomic-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-ruby.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/ruby/Ruby.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Tony Gutierrez]

> On Oct. 7, 2016, 7:15 a.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?
> 
> Andreas Hansson wrote:
> Excellent question. I think the topology line is acually both wrong and 
> unnecessary.
> 
> I really must confess the current behaviuor of addToPath is a bit of a 
> mystery (what is it really relative to), and it turns out it is relative to 
> the run.py script in the test folder.
> 
> Jason Lowe-Power wrote:
> Thanks for the info. I agree about addToPath :).

You're right, according to the comment near the definition of addToPath() it 
seems things are relative to whichever run script you're using. Unfortunately 
the relative path of configs is not the same for all run scripts. E.g., using 
apu_se.py by hand still doesn't work, even with this patch, and I have to 
manually add the paths back to apu_se.py for things to work.


- Tony


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


On Oct. 7, 2016, 10:29 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 10:29 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:8816ebf0a103
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated, and the messy path-deducing variations in
> the scripts are unified.
> 
> 
> Diffs
> -
> 
>   tests/configs/gpu-randomtest-ruby.py ebf2acd02fc5 
>   tests/configs/gpu-ruby.py ebf2acd02fc5 
>   tests/configs/memtest-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-ruby.py ebf2acd02fc5 
>   tests/configs/pc-simple-timing-ruby.py ebf2acd02fc5 
>   tests/configs/rubytest-ruby.py ebf2acd02fc5 
>   tests/configs/simple-atomic-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-ruby.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/ruby/Ruby.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Brandon Potter]

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


I'm noticing that this patch doesn't completely fix the regressions on its own. 
There are still two failures that I'm seeing with it applied (although they may 
be unrelated, not sure):

`cat 
build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_RfO/simerr`

warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'dst_node_' already has parent
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
line 400, in main
exec filecode in scope
  File 
"/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", line 
184, in 
execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
  File 
"/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-ruby.py",
 line 282, in 
Ruby.create_system(options, None, system)
  File "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
line 175, in create_system
Network.init_network(options, network, InterfaceClass)
  File 
"/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py", 
line 108, in init_network
network.setup_buffers()
  File 
"/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
 line 62, in setup_buffers
if link.dst_node == router:
  File 
"/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py", 
line 1105, in __getattr__
raise AttributeError, err_string
AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
  (C++ object is not yet constructed, so wrapped C++ methods are unavailable.)

`cat 
build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-randomtest-ruby-GPU_RfO/simerr`

warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'int_node' already has parent
warn: add_child('dst_node_'): child 'dst_node_' already has parent
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/main.py", 
line 400, in main
exec filecode in scope
  File 
"/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../run.py", line 
184, in 
execfile(joinpath(tests_root, 'configs', test_filename + '.py'))
  File 
"/home/bpotter/Research/gem5/gem5-public-process/tests/testing/../configs/gpu-randomtest-ruby.py",
 line 104, in 
Ruby.create_system(options, False, system)
  File "/home/bpotter/Research/gem5/gem5-public-process/configs/ruby/Ruby.py", 
line 175, in create_system
Network.init_network(options, network, InterfaceClass)
  File 
"/home/bpotter/Research/gem5/gem5-public-process/configs/network/Network.py", 
line 108, in init_network
network.setup_buffers()
  File 
"/home/bpotter/Research/gem5/gem5-public-process/src/mem/ruby/network/simple/SimpleNetwork.py",
 line 62, in setup_buffers
if link.dst_node == router:
  File 
"/home/bpotter/Research/gem5/gem5-public-process/src/python/m5/SimObject.py", 
line 1105, in __getattr__
raise AttributeError, err_string
AttributeError: object 'SimpleIntLink' has no attribute 'dst_node'
  (C++ object is not yet constructed, so wrapped C++ methods are unavailable.)

- Brandon Potter


On Oct. 7, 2016, 5:29 p.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 5:29 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:8816ebf0a103
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated, and the messy path-deducing variations in
> the scripts are unified.
> 
> 
> Diffs
> -
> 
>   tests/configs/gpu-randomtest-ruby.py ebf2acd02fc5 
>   tests/configs/gpu-ruby.py ebf2acd02fc5 
>   tests/configs/memtest-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/o3-timing-ruby.py ebf2acd02fc5 
>   tests/configs/pc-simple-timing-ruby.py ebf2acd02fc5 
>   tests/configs/rubytest-ruby.py ebf2acd02fc5 
>   tests/configs/simple-atomic-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-mp-ruby.py ebf2acd02fc5 
>   tests/configs/simple-timing-ruby.py 

[gem5-dev] Review Request 3649: gpu-compute: add instruction mix stats for the gpu

[Tony Gutierrez]

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11675:4ce20a015e53
---
gpu-compute: add instruction mix stats for the gpu


Diffs
-

  src/gpu-compute/compute_unit.hh 3803750858632ca6a80747881347846c0e44cba5 
  src/gpu-compute/compute_unit.cc 3803750858632ca6a80747881347846c0e44cba5 
  src/gpu-compute/wavefront.cc 3803750858632ca6a80747881347846c0e44cba5 

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


Testing
---


Thanks,

Tony Gutierrez

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

Re: [gem5-dev] Review Request 3646: gpu-compute, hsail: call discardFetch() from the WF

[Tony Gutierrez]

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

(Updated Oct. 7, 2016, 1:53 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11676:28636ac9878f
---
gpu-compute, hsail: call discardFetch() from the WF

because every taken branch causes fetch to be discarded, we move the call
to the WF to avoid to have to call it from each and every branch instruction
type.


Diffs (updated)
-

  src/arch/hsail/insts/branch.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
  src/gpu-compute/wavefront.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 

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


Testing
---


Thanks,

Tony Gutierrez

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

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

[Brandon Potter]

> On Oct. 7, 2016, 9:31 a.m., Andreas Sandberg wrote:
> > Thanks for doing this! This is something that has annoyed me for quite some 
> > time as well.
> > 
> > Thee are quite a few files modified in patch where the header order isn't 
> > compliant with the style guide, which is probably why you are seeing style 
> > checker errors. Ideally, I'd like to see the primary header first rule 
> > enforced in all CC files since that ensures that header files include all 
> > of their dependencies, but I wouldn't say that's a blocker.
> 
> Andreas Sandberg wrote:
> I have just added the necessary fixes to the style checker to automate 
> this (see [RB3648](http://reviews.gem5.org/r/3648/)). We (I'm happy to do it) 
> should be able to fix the include order automagically using this:
> 
> ./util/style.py -f -c SortedIncludes `find src/ -name *.cc`
> 

Your patch appears to fix the qref issue so thanks for posting it and for the 
quick response!

I've created a patch using the command that you've provided; it's sitting in my 
patch queue before my patches. There weren't too many merge conflicts with my 
patches which is nice. However, I've found that the new style checker's header 
reorganization exposes problems with missing includes in the header files when 
I subsequently try to compile. __Do you mind if I go through the process of 
resolving the fallout or do you want to do it?__ I don't mind doing it myself 
since it's currently blocking the header file changes that I'd like to post, 
but I don't want to steal the limelight given that you wrote the original style 
fix.

Please let me know what you want to do.


- Brandon


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


On Oct. 5, 2016, 4:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> ---
> 
> (Updated Oct. 5, 2016, 4:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11660:1c76ab8bafec
> ---
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table_impl.hh 
> b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/arguments.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/generic/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/power/interrupts.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pseudo_inst.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/system.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
> 
> Diff: http://reviews.gem5.org/r/3643/diff/
> 
> 
> Testing
> ---
> 
> util/regress
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Andreas Hansson]

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

(Updated Oct. 7, 2016, 5:29 p.m.)


Review request for Default.


Repository: gem5


Description (updated)
---

Changeset 11668:8816ebf0a103
---
ruby: Fix broken regressions

This patch moves the addition of network options into the Ruby module
to avoid the regressions all having to add it explicitly. The example
scripts are also updated, and the messy path-deducing variations in
the scripts are unified.


Diffs (updated)
-

  tests/configs/gpu-randomtest-ruby.py ebf2acd02fc5 
  tests/configs/gpu-ruby.py ebf2acd02fc5 
  tests/configs/memtest-ruby.py ebf2acd02fc5 
  tests/configs/o3-timing-mp-ruby.py ebf2acd02fc5 
  tests/configs/o3-timing-ruby.py ebf2acd02fc5 
  tests/configs/pc-simple-timing-ruby.py ebf2acd02fc5 
  tests/configs/rubytest-ruby.py ebf2acd02fc5 
  tests/configs/simple-atomic-mp-ruby.py ebf2acd02fc5 
  tests/configs/simple-timing-mp-ruby.py ebf2acd02fc5 
  tests/configs/simple-timing-ruby.py ebf2acd02fc5 
  configs/example/apu_se.py ebf2acd02fc5 
  configs/example/fs.py ebf2acd02fc5 
  configs/example/garnet_synth_traffic.py ebf2acd02fc5 
  configs/example/ruby_direct_test.py ebf2acd02fc5 
  configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
  configs/example/ruby_mem_test.py ebf2acd02fc5 
  configs/example/ruby_random_test.py ebf2acd02fc5 
  configs/example/se.py ebf2acd02fc5 
  configs/ruby/Ruby.py ebf2acd02fc5 

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


Testing
---


Thanks,

Andreas Hansson

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

Re: [gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Andreas Sandberg]

> On Oct. 7, 2016, 4:16 p.m., Jason Lowe-Power wrote:
> > util/style.py, line 117
> > 
> >
> > I'm going to be picky here... Can you change this to the below so you 
> > don't have the awkward newline when printing the help text.
> > 
> > ```
> > help="Style checkers to run. Can be specified "
> > "multiple times."
> > ```
> 
> Andreas Sandberg wrote:
> Argparse actually ignores this newline. There is a help formatter thingy 
> that removes newlines and generally cleans up indentation. We actually have a 
> custom formatter in tests/tests.py to make it print paragraphs correctly in 
> the usage epilog.
> 
> Jason Lowe-Power wrote:
> I didn't know that. Thanks for informing me.
> 
> We should pick one or the other and put it in the style guide. I see a 
> mix of both throughout the codebase. I would personally rather not use the 
> triple quotes to keep things consistent between python and C/C++. Thoughts?

I'm not too bothered about C++ and Python being consistent. In general, we 
should follow the official Python style guide (there is such a thing) for 
Python files. Otherwise, we end up with weird looking Python code. It's not 
possible to do consistently though because the C++ world "leaks" into Python.


- Andreas


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


On Oct. 7, 2016, 4:11 p.m., Andreas Sandberg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3648/
> ---
> 
> (Updated Oct. 7, 2016, 4:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11669:258c76f88f24
> ---
> style: Add options to select checkers and apply fixes
> 
> Add an option, --checker/-c, to style.py that selects individual style
> checkers to apply. When this option isn't specified, the script
> defaults to all available style checkers. The option may be specified
> multiple times to run multiple style checkers.
> 
> The option, --fix/-f, can be specified to automatically fix style
> violations.
> 
> Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
> Signed-off-by: Andreas Sandberg 
> Reviewed-by: Andreas Hansson 
> 
> 
> Diffs
> -
> 
>   util/style.py 380375085863 
> 
> Diff: http://reviews.gem5.org/r/3648/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

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

Re: [gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Jason Lowe-Power]

> On Oct. 7, 2016, 3:16 p.m., Jason Lowe-Power wrote:
> > util/style.py, line 117
> > 
> >
> > I'm going to be picky here... Can you change this to the below so you 
> > don't have the awkward newline when printing the help text.
> > 
> > ```
> > help="Style checkers to run. Can be specified "
> > "multiple times."
> > ```
> 
> Andreas Sandberg wrote:
> Argparse actually ignores this newline. There is a help formatter thingy 
> that removes newlines and generally cleans up indentation. We actually have a 
> custom formatter in tests/tests.py to make it print paragraphs correctly in 
> the usage epilog.

I didn't know that. Thanks for informing me.

We should pick one or the other and put it in the style guide. I see a mix of 
both throughout the codebase. I would personally rather not use the triple 
quotes to keep things consistent between python and C/C++. Thoughts?


- Jason


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


On Oct. 7, 2016, 3:11 p.m., Andreas Sandberg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3648/
> ---
> 
> (Updated Oct. 7, 2016, 3:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11669:258c76f88f24
> ---
> style: Add options to select checkers and apply fixes
> 
> Add an option, --checker/-c, to style.py that selects individual style
> checkers to apply. When this option isn't specified, the script
> defaults to all available style checkers. The option may be specified
> multiple times to run multiple style checkers.
> 
> The option, --fix/-f, can be specified to automatically fix style
> violations.
> 
> Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
> Signed-off-by: Andreas Sandberg 
> Reviewed-by: Andreas Hansson 
> 
> 
> Diffs
> -
> 
>   util/style.py 380375085863 
> 
> Diff: http://reviews.gem5.org/r/3648/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

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

Re: [gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Andreas Sandberg]

> On Oct. 7, 2016, 4:16 p.m., Jason Lowe-Power wrote:
> > util/style.py, line 117
> > 
> >
> > I'm going to be picky here... Can you change this to the below so you 
> > don't have the awkward newline when printing the help text.
> > 
> > ```
> > help="Style checkers to run. Can be specified "
> > "multiple times."
> > ```

Argparse actually ignores this newline. There is a help formatter thingy that 
removes newlines and generally cleans up indentation. We actually have a custom 
formatter in tests/tests.py to make it print paragraphs correctly in the usage 
epilog.


- Andreas


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


On Oct. 7, 2016, 4:11 p.m., Andreas Sandberg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3648/
> ---
> 
> (Updated Oct. 7, 2016, 4:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11669:258c76f88f24
> ---
> style: Add options to select checkers and apply fixes
> 
> Add an option, --checker/-c, to style.py that selects individual style
> checkers to apply. When this option isn't specified, the script
> defaults to all available style checkers. The option may be specified
> multiple times to run multiple style checkers.
> 
> The option, --fix/-f, can be specified to automatically fix style
> violations.
> 
> Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
> Signed-off-by: Andreas Sandberg 
> Reviewed-by: Andreas Hansson 
> 
> 
> Diffs
> -
> 
>   util/style.py 380375085863 
> 
> Diff: http://reviews.gem5.org/r/3648/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Jason Lowe-Power]

> On Oct. 7, 2016, 2:15 p.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?
> 
> Andreas Hansson wrote:
> Excellent question. I think the topology line is acually both wrong and 
> unnecessary.
> 
> I really must confess the current behaviuor of addToPath is a bit of a 
> mystery (what is it really relative to), and it turns out it is relative to 
> the run.py script in the test folder.

Thanks for the info. I agree about addToPath :).


- Jason


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


On Oct. 7, 2016, 9:40 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:fd9c744bac8b
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated.
> 
> 
> Diffs
> -
> 
>   configs/ruby/Ruby.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

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

[Andreas Sandberg]

> On Oct. 7, 2016, 10:31 a.m., Andreas Sandberg wrote:
> > Thanks for doing this! This is something that has annoyed me for quite some 
> > time as well.
> > 
> > Thee are quite a few files modified in patch where the header order isn't 
> > compliant with the style guide, which is probably why you are seeing style 
> > checker errors. Ideally, I'd like to see the primary header first rule 
> > enforced in all CC files since that ensures that header files include all 
> > of their dependencies, but I wouldn't say that's a blocker.

I have just added the necessary fixes to the style checker to automate this 
(see [RB3648](http://reviews.gem5.org/r/3648/)). We (I'm happy to do it) should 
be able to fix the include order automagically using this:

./util/style.py -f -c SortedIncludes `find src/ -name *.cc`



- Andreas


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


On Oct. 5, 2016, 5:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> ---
> 
> (Updated Oct. 5, 2016, 5:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11660:1c76ab8bafec
> ---
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table_impl.hh 
> b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/arguments.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/generic/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/power/interrupts.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pseudo_inst.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/system.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
> 
> Diff: http://reviews.gem5.org/r/3643/diff/
> 
> 
> Testing
> ---
> 
> util/regress
> 
> 
> Thanks,
> 
> Brandon Potter
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Jason Lowe-Power]

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

Ship it!


Ship It!

- Jason Lowe-Power


On Oct. 7, 2016, 9:40 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:fd9c744bac8b
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated.
> 
> 
> Diffs
> -
> 
>   configs/ruby/Ruby.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Re: [gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Jason Lowe-Power]

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

Ship it!


Minor thing below, but other than that it looks good.


util/style.py (line 117)


I'm going to be picky here... Can you change this to the below so you don't 
have the awkward newline when printing the help text.

```
help="Style checkers to run. Can be specified "
"multiple times."
```


- Jason Lowe-Power


On Oct. 7, 2016, 3:11 p.m., Andreas Sandberg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3648/
> ---
> 
> (Updated Oct. 7, 2016, 3:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11669:258c76f88f24
> ---
> style: Add options to select checkers and apply fixes
> 
> Add an option, --checker/-c, to style.py that selects individual style
> checkers to apply. When this option isn't specified, the script
> defaults to all available style checkers. The option may be specified
> multiple times to run multiple style checkers.
> 
> The option, --fix/-f, can be specified to automatically fix style
> violations.
> 
> Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
> Signed-off-by: Andreas Sandberg 
> Reviewed-by: Andreas Hansson 
> 
> 
> Diffs
> -
> 
>   util/style.py 380375085863 
> 
> Diff: http://reviews.gem5.org/r/3648/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Sandberg
> 
>

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

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Andreas Hansson]

> On Oct. 7, 2016, 2:15 p.m., Jason Lowe-Power wrote:
> > configs/ruby/Ruby.py, line 50
> > 
> >
> > I think I'm missing something, but isn't the network directory and the 
> > topologies directory both in configs/? Why are these addToPath calls 
> > different?

Excellent question. I think the topology line is acually both wrong and 
unnecessary.

I really must confess the current behaviuor of addToPath is a bit of a mystery 
(what is it really relative to), and it turns out it is relative to the run.py 
script in the test folder.


- Andreas


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


On Oct. 7, 2016, 9:40 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:fd9c744bac8b
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated.
> 
> 
> Diffs
> -
> 
>   configs/ruby/Ruby.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

[gem5-dev] changeset in gem5: dev, arm: Make GenericTimer param handling mo...

[Andreas Sandberg]

changeset 380375085863 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=380375085863
description:
dev, arm: Make GenericTimer param handling more robust

The generic timer needs a pointer to an ArmSystem to wire itself to the
system register handler. This was previously specified as an instance
of System that was later cast to ArmSystem. Make this more robust by
specifying it as an ArmSystem in the Python interface and add a check
to make sure that it is non-NULL.

Change-Id: I989455e666f4ea324df28124edbbadfd094b0d02
Signed-off-by: Andreas Sandberg 
Reviewed-by: Nikos Nikoleris 

diffstat:

 src/dev/arm/RealView.py  |  2 +-
 src/dev/arm/generic_timer.cc |  3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diffs (25 lines):

diff -r ebf2acd02fc5 -r 380375085863 src/dev/arm/RealView.py
--- a/src/dev/arm/RealView.py   Thu Oct 06 21:06:00 2016 -0400
+++ b/src/dev/arm/RealView.py   Fri Oct 07 14:14:44 2016 +0100
@@ -231,7 +231,7 @@
 class GenericTimer(SimObject):
 type = 'GenericTimer'
 cxx_header = "dev/arm/generic_timer.hh"
-system = Param.System(Parent.any, "system")
+system = Param.ArmSystem(Parent.any, "system")
 gic = Param.BaseGic(Parent.any, "GIC to use for interrupting")
 # @todo: for now only two timers per CPU is supported, which is the
 # normal behaviour when security extensions are disabled.
diff -r ebf2acd02fc5 -r 380375085863 src/dev/arm/generic_timer.cc
--- a/src/dev/arm/generic_timer.cc  Thu Oct 06 21:06:00 2016 -0400
+++ b/src/dev/arm/generic_timer.cc  Fri Oct 07 14:14:44 2016 +0100
@@ -234,7 +234,8 @@
   irqPhys(p->int_phys),
   irqVirt(p->int_virt)
 {
-dynamic_cast(*p->system).setGenericTimer(this);
+fatal_if(!p->system, "No system specified, can't instantiate timer.\n");
+p->system->setGenericTimer(this);
 }
 
 void
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request 3648: style: Add options to select checkers and apply fixes

[Andreas Sandberg]

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11669:258c76f88f24
---
style: Add options to select checkers and apply fixes

Add an option, --checker/-c, to style.py that selects individual style
checkers to apply. When this option isn't specified, the script
defaults to all available style checkers. The option may be specified
multiple times to run multiple style checkers.

The option, --fix/-f, can be specified to automatically fix style
violations.

Change-Id: Id7597fba6b65cecfa17a88b1c87c8a4c8315af59
Signed-off-by: Andreas Sandberg 
Reviewed-by: Andreas Hansson 


Diffs
-

  util/style.py 380375085863 

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


Testing
---


Thanks,

Andreas Sandberg

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

Re: [gem5-dev] Stable release

[Pierre-Yves Péneau]

I am glad to see that my email leads to quick response and action :-)
Thank you all.

On 10/07/2016 04:04 PM, Jason Lowe-Power wrote:
> Thanks for getting back to me everyone. I'm going to send out a message to
> gem5-users to make sure there are no other objections. If there's nothing
> in about a week, we should shut down gem5-stable.
> 
> Who knows how to do this / has the access to do it? I don't.
> 
> Thanks,
> Jason
> 
> On Mon, Oct 3, 2016 at 5:59 AM Andreas Sandberg 
> wrote:
> 
>> +1 for #1 from me as well.
>>
>> Ideally, I’d like to see #3, but I don’t think we can make that work at
>> the moment.
>>
>> //Andreas
>>
>> On 03/10/2016, 01:06, "gem5-dev on behalf of Steve Reinhardt" <
>> gem5-dev-boun...@gem5.org on behalf of
>> ste...@gmail.com> wrote:
>>
>> I agree with #1. The existence of gem5-stable is just confusing, since we
>> don't have a good process to keep it up-to-date.
>>
>> Steve
>>
>>
>> On Fri, Sep 30, 2016 at 8:56 AM Beckmann, Brad > >
>> wrote:
>>
>> I strongly support 1 as well.
>>
>> Brad
>>
>>
>> -Original Message-
>> From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Andreas
>> Hansson
>> Sent: Friday, September 30, 2016 7:03 AM
>> To: gem5 Developer List >
>> Subject: Re: [gem5-dev] Stable release
>>
>> Hi Jason,
>>
>> I support 1 for now (based on example the argument you mention). Once we
>> have moved to git we can reconsider.
>>
>> Andreas
>>
>> On 30/09/2016, 15:00, "gem5-dev on behalf of Jason Lowe-Power"
>>  on behalf of
>> ja...@lowepower.com> wrote:
>>
>>> Thanks for bringing this up. It's been a long time since we've
>>> discussed gem5-stable on the mailing list.
>>>
>>> I propose that we come up with a few options on what we, as a
>>> community, want to do with gem5-stable, vote on them, and then stick to
>> our decision.
>>> Below is what I see are the options. If anyone else has other ideas,
>>> please chime in!
>>>
>>> 1. Get rid of gem5-stable.
>>> 2. Continue with the previous approach of tagging gem5 every quarter
>>> and releasing a gem5-stable branch.
>>> 3. Develop a process of milestones and release after each milestone is
>>> complete. Between milestone releases, we can backport bugfixes into
>>> gem5-stable.
>>>
>>> My opinions:
>>> 1. gem5-stable is no more bug-free than gem5-dev. It's only "stable" in
>>> the sense that we never commit to it. *I think this is what we should
>>> do for
>>> now.*
>>> 2. I don't think that the previous gem5-stable approach of tagging gem5
>>> quarterly provides any added benefit. Users can just as easily clone
>>> gem5-dev and just not update if they want a "stable" platform. That's
>>> all gem5-stable was anyway.
>>> 3. I think having milestones and true "releases" would be the best
>>> thing we can do. However, I don't think we are currently at a place
>>> where we can implement this. If I'm wrong, and someone in the community
>>> wants to step up and take this responsibility, I think it would really
>>> benefit our users.
>>>
>>> What do other's think?
>>>
>>> Cheers,
>>> Jason
>>>
>>>
>>>
>>> On Fri, Sep 30, 2016 at 4:24 AM Pierre-Yves Péneau <
>>> pierre-yves.pen...@lirmm.fr> wrote:
>>>
 Hi all,

 Since September 2015, gem5 has no stable version. However, a high
 activity has been observed this year, and a lot of major features
 have been added (big.LITTLE support, power modeling, new GPUs,
 ElasticTraces etc..). I am wondering if there is any plan to release
 a new stable version in the next weeks/months ?

 Thank you.

 --
 +-+
 | Pierre-Yves Péneau - PhD student |  first.last at lirmm.fr  |
 | LIRMM / CNRS - SYSMIC team   |+ 33 4 67 41 86 33
>> <+33%204%2067%2041%2086%2033>|
 | Building 4 Office H2.2   |http://walafc0.org|
 +-+
 ___
 gem5-dev mailing list
 gem5-dev@gem5.org
 http://m5sim.org/mailman/listinfo/gem5-dev

>>> ___
>>> gem5-dev mailing list
>>> gem5-dev@gem5.org
>>> http://m5sim.org/mailman/listinfo/gem5-dev
>>
>> IMPORTANT NOTICE: The contents of this email and any attachments are
>> confidential and may also be privileged. If you are not the intended
>> recipient, please notify the sender immediately and do not disclose the
>> contents to any other person, use it for any purpose, or store or copy the
>> information in any medium. Thank you.
>> ___
>> gem5-dev mailing 

Re: [gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Jason Lowe-Power]

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



configs/ruby/Ruby.py (line 50)


I think I'm missing something, but isn't the network directory and the 
topologies directory both in configs/? Why are these addToPath calls different?


- Jason Lowe-Power


On Oct. 7, 2016, 9:40 a.m., Andreas Hansson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3647/
> ---
> 
> (Updated Oct. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11668:fd9c744bac8b
> ---
> ruby: Fix broken regressions
> 
> This patch moves the addition of network options into the Ruby module
> to avoid the regressions all having to add it explicitly. The example
> scripts are also updated.
> 
> 
> Diffs
> -
> 
>   configs/ruby/Ruby.py ebf2acd02fc5 
>   configs/example/ruby_random_test.py ebf2acd02fc5 
>   configs/example/se.py ebf2acd02fc5 
>   configs/example/ruby_direct_test.py ebf2acd02fc5 
>   configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
>   configs/example/ruby_mem_test.py ebf2acd02fc5 
>   configs/example/apu_se.py ebf2acd02fc5 
>   configs/example/fs.py ebf2acd02fc5 
>   configs/example/garnet_synth_traffic.py ebf2acd02fc5 
> 
> Diff: http://reviews.gem5.org/r/3647/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andreas Hansson
> 
>

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

Re: [gem5-dev] Stable release

[Jason Lowe-Power]

Thanks for getting back to me everyone. I'm going to send out a message to
gem5-users to make sure there are no other objections. If there's nothing
in about a week, we should shut down gem5-stable.

Who knows how to do this / has the access to do it? I don't.

Thanks,
Jason

On Mon, Oct 3, 2016 at 5:59 AM Andreas Sandberg 
wrote:

> +1 for #1 from me as well.
>
> Ideally, I’d like to see #3, but I don’t think we can make that work at
> the moment.
>
> //Andreas
>
> On 03/10/2016, 01:06, "gem5-dev on behalf of Steve Reinhardt" <
> gem5-dev-boun...@gem5.org on behalf of
> ste...@gmail.com> wrote:
>
> I agree with #1. The existence of gem5-stable is just confusing, since we
> don't have a good process to keep it up-to-date.
>
> Steve
>
>
> On Fri, Sep 30, 2016 at 8:56 AM Beckmann, Brad  >
> wrote:
>
> I strongly support 1 as well.
>
> Brad
>
>
> -Original Message-
> From: gem5-dev [mailto:gem5-dev-boun...@gem5.org] On Behalf Of Andreas
> Hansson
> Sent: Friday, September 30, 2016 7:03 AM
> To: gem5 Developer List >
> Subject: Re: [gem5-dev] Stable release
>
> Hi Jason,
>
> I support 1 for now (based on example the argument you mention). Once we
> have moved to git we can reconsider.
>
> Andreas
>
> On 30/09/2016, 15:00, "gem5-dev on behalf of Jason Lowe-Power"
>  on behalf of
> ja...@lowepower.com> wrote:
>
> >Thanks for bringing this up. It's been a long time since we've
> >discussed gem5-stable on the mailing list.
> >
> >I propose that we come up with a few options on what we, as a
> >community, want to do with gem5-stable, vote on them, and then stick to
> our decision.
> >Below is what I see are the options. If anyone else has other ideas,
> >please chime in!
> >
> >1. Get rid of gem5-stable.
> >2. Continue with the previous approach of tagging gem5 every quarter
> >and releasing a gem5-stable branch.
> >3. Develop a process of milestones and release after each milestone is
> >complete. Between milestone releases, we can backport bugfixes into
> >gem5-stable.
> >
> >My opinions:
> >1. gem5-stable is no more bug-free than gem5-dev. It's only "stable" in
> >the sense that we never commit to it. *I think this is what we should
> >do for
> >now.*
> >2. I don't think that the previous gem5-stable approach of tagging gem5
> >quarterly provides any added benefit. Users can just as easily clone
> >gem5-dev and just not update if they want a "stable" platform. That's
> >all gem5-stable was anyway.
> >3. I think having milestones and true "releases" would be the best
> >thing we can do. However, I don't think we are currently at a place
> >where we can implement this. If I'm wrong, and someone in the community
> >wants to step up and take this responsibility, I think it would really
> >benefit our users.
> >
> >What do other's think?
> >
> >Cheers,
> >Jason
> >
> >
> >
> >On Fri, Sep 30, 2016 at 4:24 AM Pierre-Yves Péneau <
> >pierre-yves.pen...@lirmm.fr> wrote:
> >
> >> Hi all,
> >>
> >> Since September 2015, gem5 has no stable version. However, a high
> >> activity has been observed this year, and a lot of major features
> >> have been added (big.LITTLE support, power modeling, new GPUs,
> >> ElasticTraces etc..). I am wondering if there is any plan to release
> >> a new stable version in the next weeks/months ?
> >>
> >> Thank you.
> >>
> >> --
> >> +-+
> >> | Pierre-Yves Péneau - PhD student |  first.last at lirmm.fr  |
> >> | LIRMM / CNRS - SYSMIC team   |+ 33 4 67 41 86 33
> <+33%204%2067%2041%2086%2033>|
> >> | Building 4 Office H2.2   |http://walafc0.org|
> >> +-+
> >> ___
> >> gem5-dev mailing list
> >> gem5-dev@gem5.org
> >> http://m5sim.org/mailman/listinfo/gem5-dev
> >>
> >___
> >gem5-dev mailing list
> >gem5-dev@gem5.org
> >http://m5sim.org/mailman/listinfo/gem5-dev
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> ___
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
> ___
> gem5-dev mailing list
> gem5-dev@gem5.org
> 

[gem5-dev] Review Request 3647: ruby: Fix broken regressions

[Andreas Hansson]

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11668:fd9c744bac8b
---
ruby: Fix broken regressions

This patch moves the addition of network options into the Ruby module
to avoid the regressions all having to add it explicitly. The example
scripts are also updated.


Diffs
-

  configs/ruby/Ruby.py ebf2acd02fc5 
  configs/example/ruby_random_test.py ebf2acd02fc5 
  configs/example/se.py ebf2acd02fc5 
  configs/example/ruby_direct_test.py ebf2acd02fc5 
  configs/example/ruby_gpu_random_test.py ebf2acd02fc5 
  configs/example/ruby_mem_test.py ebf2acd02fc5 
  configs/example/apu_se.py ebf2acd02fc5 
  configs/example/fs.py ebf2acd02fc5 
  configs/example/garnet_synth_traffic.py ebf2acd02fc5 

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


Testing
---


Thanks,

Andreas Hansson

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

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

[Andreas Sandberg]

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

Ship it!


Thanks for doing this! This is something that has annoyed me for quite some 
time as well.

Thee are quite a few files modified in patch where the header order isn't 
compliant with the style guide, which is probably why you are seeing style 
checker errors. Ideally, I'd like to see the primary header first rule enforced 
in all CC files since that ensures that header files include all of their 
dependencies, but I wouldn't say that's a blocker.

- Andreas Sandberg


On Oct. 5, 2016, 5:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> ---
> 
> (Updated Oct. 5, 2016, 5:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11660:1c76ab8bafec
> ---
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/utility.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/kern/linux/linux.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/multi_level_page_table_impl.hh 
> b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/mem/page_table.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/arguments.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/process.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/sim_object.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/sim/syscall_emul.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/generic/tlb.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/power/interrupts.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pagetable.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/pseudo_inst.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>   src/arch/x86/system.hh b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
> 
> Diff: http://reviews.gem5.org/r/3643/diff/
> 
> 
> Testing
> ---
> 
> util/regress
> 
> 
> 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: reduce include dependencies in some headers

[Andreas Sandberg]

> On Oct. 6, 2016, 3:57 p.m., Jason Lowe-Power wrote:
> > Seems reasonable to me. Could you explain in the commit message how you 
> > decided which headers to remove/keep/add? Without digging into these files 
> > it's hard to understand your thought process.
> > 
> > Related: Do you think it makes sense to codify which headers to include in 
> > the style guideline? IMO, this would be helpful so you don't have to go 
> > through and do this again :).
> > 
> > One other small thing. I don't think we should hold up this patch for this 
> > issue, but it's something that's been bothering me. It seems the style 
> > checker is complaining about the ordering of includes for this patch, 
> > though I don't think they are actually wrong. When I run the following I 
> > get errors:
> > 
> > ```
> > hg qref COPYING # to get the patch to be "unapplied" but the changes 
> > reflected to the files
> > util/style.py 
> > ... lots of errors. For every single file it complains about the ordering 
> > of includes.
> > ```
> > 
> > If I just use hg qref, the style checker doesn't complain. Ideas?
> 
> Brandon Potter wrote:
> I think that the style checker is buggy. It's kind of beautiful (or maybe 
> completely disgusting), but my current patch queue has a patch that refuses 
> to ever properly resolve an "hg qref". I run "qref" once and it resolves with 
> a single issue about wanting to fix a line. If I try to run it again without 
> making any changes, it decides that it's still not happy and wants to make 
> more changes. This continues on ad nauseum. So yeah, beauty.
> 
> Regarding your comment about general header guidelines, developers should 
> try to do a better job about checking what their includes actually include. I 
> don't think this is enforceable just with my complaint; we need to cook this 
> into the check-in process if we want it to stick.
> 
> I've been aware that the include problem has been around for a while, but 
> I've largely ignored it due to lack of motivation about tackling a problem 
> that no one probably cares about (and would probably just cause me the 
> headache or coding it up to have it rejected). However, I ran into an issue 
> where I added a new dependency between headers files recently. It was kind of 
> interesting to find out that it involved a circular depenendcy between header 
> files between 4-5 headers. Each header had included another header and 
> eventually created a chain. (I think that it was a bit more convoluted in 
> reality and I may not have even understood it correctly, but there's a 
> definite problem here.)
> 
> My first pass, that you reviewed, was done manually. I opened header 
> files (and their included header files) and tried to figure out where things 
> are coming from. I focused mainly on the "sim" directory, because that's 
> where most of my work has been done. I posted the results after resolving 
> some of the problems and that's what you have now.
> 
> While working on this problem, I figured that this is probably a more 
> general problem for any large project. I Googled around and found a tool that 
> can give recommendations on header dependencies: cppclean. I've been using 
> the tool to try to resolve some of the issues mainly focusing on using 
> forward declarations where possible. It's pretty time consuming given that I 
> need this to work for all of the ISAs and their Ruby protocols. I've gone 
> through "src/sim" completely now with the tool and have resolved everything 
> that wasn't a false positive; the tool doesn't handle macros very well (and 
> our build process is wonky given that it uses swig). __It's really time 
> consuming; I don't know that I'm going to pursue it any further unless people 
> feel that it's worthwhile.__ I'm under constraints here at AMD to post 
> unrelated patches so I doubt that I'll have much support in my own corner to 
> spend much time on this. I'm mainly looking at ARM and Wisconsin folks to see 
> what their opinions are regarding the headers. If it's a widespread problem 
> that people want fixed, I can try to see if I can pursue this for the 
> remaining directories in "src".
> 
> Anyways, the patch is going through regressions now. I'll post the update 
> when it finishes.

I think I know what's going on here. I suspect that the include order checker 
doesn't get enough context in one of the cases and doesn't spot blocks with 
incorrect include order. In particular, the primary header first rule (i.e., 
foo.cc should include foo.hh first) tends to get confused. The solution to this 
is likely to always check the entire file in this particular checker. We 
probably want to apply the sorted include checker to all of the files in the 
source tree first though.


- Andreas


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

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

[Cron Daemon]

* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby: 
FAILED!
* build/ALPHA/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby: 
FAILED!
* build/ALPHA/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby: 
FAILED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby: 
FAILED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer:
 FAILED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer:
 FAILED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer:
 FAILED!
* 
build/ALPHA_MOESI_hammer/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer:
 FAILED!
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MESI_Two_Level:
 FAILED!
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MESI_Two_Level:
 FAILED!
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MESI_Two_Level:
 FAILED!
* 
build/ALPHA_MESI_Two_Level/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MESI_Two_Level:
 FAILED!
* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory:
 FAILED!
* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory:
 FAILED!
*** diff[simerr]: SKIPPED* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory:
 FAILED!
* 
build/ALPHA_MOESI_CMP_directory/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory:
 FAILED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token:
 FAILED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token:
 FAILED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token:
 FAILED!
* 
build/ALPHA_MOESI_CMP_token/tests/opt/quick/se/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token:
 FAILED!
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing-ruby: 
FAILED!
* build/SPARC/tests/opt/quick/se/00.hello/sparc/linux/simple-timing-ruby: 
FAILED!
* build/X86/tests/opt/quick/se/00.hello/x86/linux/simple-timing-ruby: 
FAILED!
* 
build/HSAIL_X86/tests/opt/quick/se/60.gpu-randomtest/x86/linux/gpu-randomtest-ruby-GPU_RfO:
 FAILED!
* build/HSAIL_X86/tests/opt/quick/se/04.gpu/x86/linux/gpu-ruby-GPU_RfO: 
FAILED!
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/minor-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-atomic: passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic: 
passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/minor-timing: passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-two-level:
 passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-atomic: 
passed.
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-atomic: passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-timing-dual:
 passed.
* 
build/ALPHA/tests/opt/quick/se/03.learning-gem5/alpha/linux/learning-gem5-p1-simple:
 passed.
* build/ALPHA/tests/opt/quick/se/01.hello-2T-smt/alpha/linux/o3-timing-mt: 
passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/tru64/o3-timing: passed.
* build/ALPHA/tests/opt/quick/se/00.hello/alpha/linux/simple-atomic: passed.
* build/ALPHA/tests/opt/quick/se/50.vortex/alpha/tru64/simple-timing: 
passed.
* 
build/ALPHA/tests/opt/quick/fs/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual:
 passed.
* build/ALPHA/tests/opt/quick/se/70.twolf/alpha/tru64/simple-timing: passed.
* build/ALPHA/tests/opt/quick/se/30.eon/alpha/tru64/simple-atomic: passed.
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-two-level:
 passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-timing: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/simple-atomic: passed.
* build/MIPS/tests/opt/quick/se/00.hello/mips/linux/o3-timing: passed. [
 CXX] NULL/mem/ruby/network/garnet2.0/SwitchAllocator.cc -> .o
* 
build/MIPS/tests/opt/quick/se/03.learning-gem5/mips/linux/learning-gem5-p1-simple:
 passed.
* 

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

[Brandon Potter]

> On Oct. 6, 2016, 2:57 p.m., Jason Lowe-Power wrote:
> > Seems reasonable to me. Could you explain in the commit message how you 
> > decided which headers to remove/keep/add? Without digging into these files 
> > it's hard to understand your thought process.
> > 
> > Related: Do you think it makes sense to codify which headers to include in 
> > the style guideline? IMO, this would be helpful so you don't have to go 
> > through and do this again :).
> > 
> > One other small thing. I don't think we should hold up this patch for this 
> > issue, but it's something that's been bothering me. It seems the style 
> > checker is complaining about the ordering of includes for this patch, 
> > though I don't think they are actually wrong. When I run the following I 
> > get errors:
> > 
> > ```
> > hg qref COPYING # to get the patch to be "unapplied" but the changes 
> > reflected to the files
> > util/style.py 
> > ... lots of errors. For every single file it complains about the ordering 
> > of includes.
> > ```
> > 
> > If I just use hg qref, the style checker doesn't complain. Ideas?

I think that the style checker is buggy. It's kind of beautiful (or maybe 
completely disgusting), but my current patch queue has a patch that refuses to 
ever properly resolve an "hg qref". I run "qref" once and it resolves with a 
single issue about wanting to fix a line. If I try to run it again without 
making any changes, it decides that it's still not happy and wants to make more 
changes. This continues on ad nauseum. So yeah, beauty.

Regarding your comment about general header guidelines, developers should try 
to do a better job about checking what their includes actually include. I don't 
think this is enforceable just with my complaint; we need to cook this into the 
check-in process if we want it to stick.

I've been aware that the include problem has been around for a while, but I've 
largely ignored it due to lack of motivation about tackling a problem that no 
one probably cares about (and would probably just cause me the headache or 
coding it up to have it rejected). However, I ran into an issue where I added a 
new dependency between headers files recently. It was kind of interesting to 
find out that it involved a circular depenendcy between header files between 
4-5 headers. Each header had included another header and eventually created a 
chain. (I think that it was a bit more convoluted in reality and I may not have 
even understood it correctly, but there's a definite problem here.)

My first pass, that you reviewed, was done manually. I opened header files (and 
their included header files) and tried to figure out where things are coming 
from. I focused mainly on the "sim" directory, because that's where most of my 
work has been done. I posted the results after resolving some of the problems 
and that's what you have now.

While working on this problem, I figured that this is probably a more general 
problem for any large project. I Googled around and found a tool that can give 
recommendations on header dependencies: cppclean. I've been using the tool to 
try to resolve some of the issues mainly focusing on using forward declarations 
where possible. It's pretty time consuming given that I need this to work for 
all of the ISAs and their Ruby protocols. I've gone through "src/sim" 
completely now with the tool and have resolved everything that wasn't a false 
positive; the tool doesn't handle macros very well (and our build process is 
wonky given that it uses swig). __It's really time consuming; I don't know that 
I'm going to pursue it any further unless people feel that it's worthwhile.__ 
I'm under constraints here at AMD to post unrelated patches so I doubt that 
I'll have much support in my own corner to spend much time on this. I'm mainly 
looking at ARM and Wisconsin folks to see what their opinions are regarding the 
headers. If it's a widespread problem that people want fixed, I can try to see 
if I can pursue this for the remaining directories in "src".

Anyways, the patch is going through regressions now. I'll post the update when 
it finishes.


- Brandon


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


On Oct. 5, 2016, 4:27 p.m., Brandon Potter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3643/
> ---
> 
> (Updated Oct. 5, 2016, 4:27 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11660:1c76ab8bafec
> ---
> style: reduce include dependencies in some headers
> 
> 
> Diffs
> -
> 
>   src/arch/x86/system.cc b29aca3fcb75f5ad92429001ab11c65b2f9635b0 
>