[m5-dev] Cron m5t...@zizzer /z/m5/regression/do-regression quick
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing passed. * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp passed. * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic passed. * build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing passed. * build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing passed. * build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby passed. * build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby passed. * build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing passed. * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer passed. * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer passed. * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer passed. * build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer passed. * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby passed. * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory passed. * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory passed. * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory passed. * build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory passed. * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory passed. * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory passed. * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory passed. * build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory passed. * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token passed. * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token passed. * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token passed. * build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token passed. * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing-dual passed. * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing passed. * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual passed. * build/ALPHA_FS/tests/fast/quick/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic passed. * build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic passed. * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-atomic passed. * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing passed. * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing-ruby passed. * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/o3-timing passed. * build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/inorder-timing passed. * build/POWER_SE/tests/fast/quick/00.hello/power/linux/simple-atomic passed. * build/POWER_SE/tests/fast/quick/00.hello/power/linux/o3-timing passed. * build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest passed. * build/SPARC_SE/tests/fast/quick/40.m5threads-test-atomic/sparc/linux/simple-timing-mp passed. * build/SPARC_SE/tests/fast/quick/02.insttest/sparc/linux/simple-timing passed. * build/SPARC_SE/tests/fast/quick/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp passed. * build/SPARC_SE/tests/fast/quick/00.hello/sparc/linux/simple-timing passed. * build/SPARC_SE/tests/fast/quick/02.insttest/sparc/linux/simple-atomic passed. * build/SPARC_SE/tests/fast/quick/00.hello/sparc/linux/simple-timing-ruby passed. *
Re: [m5-dev] Review Request: Add seed option to ruby_random_test.py
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/361/#review584 --- Hi Nilay, This feature is long overdue, but I believe it is best if we add the command line to Ruby.py. That way all ruby configuration, including ruby_random_test, can use this feature. Brad - Brad On 2010-12-29 09:18:47, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/361/ --- (Updated 2010-12-29 09:18:47) Review request for Default. Summary --- This patch adds an option to the script ruby_random_test.py for setting the parameter m_random_seed used for randomizing delays in the memory system. Diffs - configs/example/ruby_random_test.py fbf4b1b18202 Diff: http://reviews.m5sim.org/r/361/diff Testing --- The number of cycles required for executing a particular number of loads changes with different seeds. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Add seed option to ruby_random_test.py
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/361/ --- (Updated 2011-01-03 09:54:30.940063) Review request for Default. Changes --- Brad, I was waiting for you to comment on where you would prefer this option to go. I have made the change. I did some minimal testing using ruby_random_test.py. The number of ticks for a fixed number of loads change when I supply different numbers as random seeds. Summary --- This patch adds an option to the script ruby_random_test.py for setting the parameter m_random_seed used for randomizing delays in the memory system. Diffs (updated) - configs/ruby/Ruby.py UNKNOWN Diff: http://reviews.m5sim.org/r/361/diff Testing --- The number of cycles required for executing a particular number of loads changes with different seeds. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/#review585 --- src/mem/ruby/slicc_interface/AbstractCacheEntry.hh http://reviews.m5sim.org/r/358/#comment823 It looks like you're also changing the lock flag from being a separate table to a cache entry field... this seems reasonable, but also seems independent of the other changes. I don't know that it's worth separating into separate changeset, but there should be some mention of this in the commit message I think. src/mem/slicc/ast/ActionDeclAST.py http://reviews.m5sim.org/r/358/#comment822 The expression '%s % foo' is a pretty unconventional way to force foo to be a string :-). If foo is already a string (which might be the case here) you don't need it at all. If foo is not a string, the conventional way to force a conversion is 'str(foo)'. I see there are a number of places where you do this. - Steve On 2010-12-31 17:50:59, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/ --- (Updated 2010-12-31 17:50:59) Review request for Default. Summary --- The purpose of this patch is to change the way CacheMemory interfaces with coherence protocols. Currently, whenever a cache controller (defined in the protocol under consideration) needs to carry out any operation on a cache block, it looks up the tag hash map and figures out whether or not the block exists in the cache. In case it does exist, the operation is carried out (which requires another lookup). Over a single clock cycle, multiple such lookups take place as observed through profiling of different protocols. It was seen that the tag lookup takes anything from 10% to 20% of the simulation time. In order to reduce this time, this patch is being posted. The CacheMemory class now will have a function that will return pointer to a cache block entry, instead of a reference (though the function that returns the reference has been retained currently). Functions have been introduced for setting/unsetting a pointer and check its pointer. Similar changes have been carried out for Transaction Buffer entries as well. Note that changes are required to both SLICC and the protocol files. This patch carries out changes to SLICC and committing this patch alone, I believe, will ___break___ all the protocols. I am working on patching the protocols as well. This patch is being put to get feed back from other developers. Diffs - src/mem/protocol/RubySlicc_Types.sm UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.hh UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.cc UNKNOWN src/mem/ruby/system/CacheMemory.hh UNKNOWN src/mem/ruby/system/CacheMemory.cc UNKNOWN src/mem/ruby/system/TBETable.hh UNKNOWN src/mem/slicc/ast/ActionDeclAST.py UNKNOWN src/mem/slicc/ast/FormalParamAST.py UNKNOWN src/mem/slicc/ast/FuncCallExprAST.py UNKNOWN src/mem/slicc/ast/FuncDeclAST.py UNKNOWN src/mem/slicc/ast/InPortDeclAST.py UNKNOWN src/mem/slicc/ast/IsValidPtrExprAST.py PRE-CREATION src/mem/slicc/ast/MethodCallExprAST.py UNKNOWN src/mem/slicc/ast/StaticCastAST.py UNKNOWN src/mem/slicc/ast/TypeDeclAST.py UNKNOWN src/mem/slicc/ast/__init__.py UNKNOWN src/mem/slicc/parser.py UNKNOWN src/mem/slicc/symbols/StateMachine.py UNKNOWN src/mem/slicc/symbols/SymbolTable.py UNKNOWN Diff: http://reviews.m5sim.org/r/358/diff Testing --- I have tested these changes using the MOESI_CMP_directory and MI protocols. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Ruby: Update MOESI CMP token protocol
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/#review588 --- src/mem/protocol/MOESI_CMP_token-L1cache.sm http://reviews.m5sim.org/r/326/#comment825 These functions that take multiple pointers just so the called function can test which one is valid are pretty unwieldy and also seem to cause a lot of redundant testing. Can we factor out the check so we have a single test to set one valid entry_ptr variable, then use that in all these calls? I see there are some similar calls in MOESI_CMP_directory too, though not as many. - Steve On 2010-12-31 17:33:04, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/ --- (Updated 2010-12-31 17:33:04) Review request for Default. Summary --- This request for reviewing the updates to implementation of the MOESI CMP token protocol. These updates have been carried out so as to conform with the changes made to CacheMemory and TBETable classes, and to SLICC. Diffs - src/mem/protocol/MOESI_CMP_token-L1cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-L2cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-dir.sm UNKNOWN Diff: http://reviews.m5sim.org/r/326/diff Testing --- Changes have been tested using ruby_random_test.py for 1,000,000 loads and 20 different seeds for random number generator. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Updating MOESI CMP Directory protocol as per the new interface
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/359/#review586 --- src/mem/protocol/MOESI_CMP_directory-L2cache.sm http://reviews.m5sim.org/r/359/#comment824 As usual, please delete unused code instead of commenting it out. - Steve On 2010-12-31 17:35:16, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/359/ --- (Updated 2010-12-31 17:35:16) Review request for Default. Summary --- This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC. Diffs - src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN Diff: http://reviews.m5sim.org/r/359/diff Testing --- These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Ruby: Update MOESI CMP token protocol
On 2011-01-03 10:14:44, Steve Reinhardt wrote: src/mem/protocol/MOESI_CMP_token-L1cache.sm, line 972 http://reviews.m5sim.org/r/326/diff/2/?file=8135#file8135line972 These functions that take multiple pointers just so the called function can test which one is valid are pretty unwieldy and also seem to cause a lot of redundant testing. Can we factor out the check so we have a single test to set one valid entry_ptr variable, then use that in all these calls? I see there are some similar calls in MOESI_CMP_directory too, though not as many. You should check my comment in the MOESI Hammer request why we cannot set just one valid entry pointer. I am of the view that we should generate the arguments for getCacheEntry() internally in SLICC. - Nilay --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/#review588 --- On 2010-12-31 17:33:04, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/ --- (Updated 2010-12-31 17:33:04) Review request for Default. Summary --- This request for reviewing the updates to implementation of the MOESI CMP token protocol. These updates have been carried out so as to conform with the changes made to CacheMemory and TBETable classes, and to SLICC. Diffs - src/mem/protocol/MOESI_CMP_token-L1cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-L2cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-dir.sm UNKNOWN Diff: http://reviews.m5sim.org/r/326/diff Testing --- Changes have been tested using ruby_random_test.py for 1,000,000 loads and 20 different seeds for random number generator. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Ruby: Update MOESI Hammer protocol
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/280/#review590 --- src/mem/protocol/MOESI_hammer-cache.sm http://reviews.m5sim.org/r/280/#comment826 There's a red blotch in reviewboard that I think is saying you have trailing space on this line. src/mem/protocol/MOESI_hammer-cache.sm http://reviews.m5sim.org/r/280/#comment828 More functions where it would be nice to identify the one valid entry_ptr up front rather than passing all of the candidates in every time. Maybe this could even be pushed all the way back up to the set_cache_entry calls? I don't know enough about the protocols to know if that's feasible, but it sounds nice. - Steve On 2010-12-31 17:26:11, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/280/ --- (Updated 2010-12-31 17:26:11) Review request for Default. Summary --- This request is for reviewing the updates made to the MOESI Hammer protocol so that it conforms with the new interfaces of CacheMemory and TBETable classes, and the changes in SLICC. Diffs - src/mem/protocol/MOESI_hammer-cache.sm UNKNOWN src/mem/protocol/MOESI_hammer-dir.sm UNKNOWN Diff: http://reviews.m5sim.org/r/280/diff Testing --- The changes have been tested using ruby_random_test.py for a 1,000,000 loads and 20 different seed values. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] changeset in m5: Ruby: Add option for random seed to Ruby.py
changeset 9d94b886c61b in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=9d94b886c61b description: Ruby: Add option for random seed to Ruby.py This patch adds an option to the script Ruby.py for setting the parameter m_random_seed used for randomizing delays in the memory system. The option can be specified as --random_seed seed value. diffstat: configs/ruby/Ruby.py | 6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diffs (22 lines): diff -r 2844d69d03e2 -r 9d94b886c61b configs/ruby/Ruby.py --- a/configs/ruby/Ruby.py Thu Dec 30 12:51:04 2010 -0500 +++ b/configs/ruby/Ruby.py Mon Jan 03 12:40:31 2011 -0600 @@ -56,7 +56,10 @@ parser.add_option(--recycle-latency, type=int, default=10, help=Recycle latency for ruby controller input buffers) - + +parser.add_option(--random_seed, type=int, default=1234, + help=Used for seeding the random number generator) + protocol = buildEnv['PROTOCOL'] exec import %s % protocol eval(%s.define_options(parser) % protocol) @@ -135,5 +138,6 @@ mem_size = total_mem_size) ruby.cpu_ruby_ports = cpu_sequencers +ruby.random_seed= options.random_seed return ruby ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] changeset in m5: RefCount: Fix reference counting pointer == and...
changeset 3a790012d6ed in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=3a790012d6ed
description:
RefCount: Fix reference counting pointer == and != with a T* on the
left.
These operators were expecting a const T instead of a const T*, and
were not
being picked up and used by gcc in the right places as a result.
Apparently no
one used these operators before. A unit test which exposed these
problems,
verified the solution, and checks other basic functionality is on the
way.
diffstat:
src/base/refcnt.hh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diffs (21 lines):
diff -r 9d94b886c61b -r 3a790012d6ed src/base/refcnt.hh
--- a/src/base/refcnt.hhMon Jan 03 12:40:31 2011 -0600
+++ b/src/base/refcnt.hhMon Jan 03 15:31:20 2011 -0500
@@ -109,7 +109,7 @@
{ return l.get() == r; }
templateclass T
-bool operator==(const T l, const RefCountingPtrT r)
+bool operator==(const T *l, const RefCountingPtrT r)
{ return l == r.get(); }
templateclass T
@@ -121,7 +121,7 @@
{ return l.get() != r; }
templateclass T
-bool operator!=(const T l, const RefCountingPtrT r)
+bool operator!=(const T *l, const RefCountingPtrT r)
{ return l != r.get(); }
#endif // __BASE_REFCNT_HH__
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/365/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- RefCount: Add a unit test for reference counting pointers. This test exercises each of the functions in the reference counting pointer implementation individually (except get()) and verifies they have some minimially expected behavior. It also checks that reference counted objects are freed when their usage count goes to 0 in some basic situations, specifically a pointer being set to NULL and a pointer being deleted. Diffs - src/unittest/SConscript 3a790012d6ed src/unittest/refcnttest.cc PRE-CREATION Diff: http://reviews.m5sim.org/r/365/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/365/ --- (Updated 2011-01-03 12:56:11.143277) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- RefCount: Add a unit test for reference counting pointers. This test exercises each of the functions in the reference counting pointer implementation individually (except get()) and verifies they have some minimially expected behavior. It also checks that reference counted objects are freed when their usage count goes to 0 in some basic situations, specifically a pointer being set to NULL and a pointer being deleted. Diffs (updated) - src/unittest/SConscript 3a790012d6ed src/unittest/refcnttest.cc PRE-CREATION Diff: http://reviews.m5sim.org/r/365/diff Testing --- Thanks, Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Updates MI cache coherence protocol
On 2011-01-03 10:16:00, Steve Reinhardt wrote: Though it was nice to split out the SLICC and protocol patches for reviewing, I think these should be combined into a single changeset for final commit. I will commit all of these together. - Nilay --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/335/#review589 --- On 2010-12-31 17:28:19, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/335/ --- (Updated 2010-12-31 17:28:19) Review request for Default. Summary --- This is a review request for the patch that updates the MI cache coherence protocol to conform with the new interfaces of CacheMemory and TBETable classes, and the changes in SLICC. Diffs - src/mem/protocol/MI_example-cache.sm UNKNOWN src/mem/protocol/MI_example-dir.sm UNKNOWN Diff: http://reviews.m5sim.org/r/335/diff Testing --- Tested using ruby_random_tester.py with l = 10,000,000 and n = 2. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/ --- (Updated 2011-01-03 14:18:13.801375) Review request for Default. Changes --- I have made the changes that Steve suggested for converting an expression in to a string. Summary --- The purpose of this patch is to change the way CacheMemory interfaces with coherence protocols. Currently, whenever a cache controller (defined in the protocol under consideration) needs to carry out any operation on a cache block, it looks up the tag hash map and figures out whether or not the block exists in the cache. In case it does exist, the operation is carried out (which requires another lookup). Over a single clock cycle, multiple such lookups take place as observed through profiling of different protocols. It was seen that the tag lookup takes anything from 10% to 20% of the simulation time. In order to reduce this time, this patch is being posted. The CacheMemory class now will have a function that will return pointer to a cache block entry, instead of a reference (though the function that returns the reference has been retained currently). Functions have been introduced for setting/unsetting a pointer and check its pointer. Similar changes have been carried out for Transaction Buffer entries as well. Note that changes are required to both SLICC and the protocol files. This patch carries out changes to SLICC and committing this patch alone, I believe, will ___break___ all the protocols. I am working on patching the protocols as well. This patch is being put to get feed back from other developers. Diffs (updated) - src/mem/protocol/RubySlicc_Types.sm UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.hh UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.cc UNKNOWN src/mem/ruby/system/CacheMemory.hh UNKNOWN src/mem/ruby/system/CacheMemory.cc UNKNOWN src/mem/ruby/system/TBETable.hh UNKNOWN src/mem/slicc/ast/ActionDeclAST.py UNKNOWN src/mem/slicc/ast/FormalParamAST.py UNKNOWN src/mem/slicc/ast/FuncCallExprAST.py UNKNOWN src/mem/slicc/ast/InPortDeclAST.py UNKNOWN src/mem/slicc/ast/MethodCallExprAST.py UNKNOWN src/mem/slicc/ast/TypeDeclAST.py UNKNOWN src/mem/slicc/ast/__init__.py UNKNOWN src/mem/slicc/parser.py UNKNOWN src/mem/slicc/symbols/StateMachine.py UNKNOWN Diff: http://reviews.m5sim.org/r/358/diff Testing --- I have tested these changes using the MOESI_CMP_directory and MI protocols. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Updating MOESI CMP Directory protocol as per the new interface
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/359/ --- (Updated 2011-01-03 14:20:22.706038) Review request for Default. Changes --- Removed the code that was commented out in the previous revision. Summary --- This is a request for reviewing the proposed changes to the MOESI CMP directory cache coherence protocol to make it conform with the new cache memory interface and changes to SLICC. Diffs (updated) - src/mem/protocol/MOESI_CMP_directory-L1cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-L2cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-dir.sm UNKNOWN src/mem/protocol/MOESI_CMP_directory-dma.sm UNKNOWN Diff: http://reviews.m5sim.org/r/359/diff Testing --- These changes have been tested using the Ruby random tester. The tester was used with -l = 1048576 and -n = 2. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] changeset in m5: Make commenting on close namespace brackets con...
changeset a8fc35183c10 in /z/repo/m5 details: http://repo.m5sim.org/m5?cmd=changeset;node=a8fc35183c10 description: Make commenting on close namespace brackets consistent. Ran all the source files through 'perl -pi' with this script: s|\s*(};?\s*)?/\*\s*(end\s*)?namespace\s*(\S+)\s*\*/(\s*})?|} // namespace $3|; s|\s*};?\s*//\s*(end\s*)?namespace\s*(\S+)\s*|} // namespace $2\n|; s|\s*};?\s*//\s*(\S+)\s*namespace\s*|} // namespace $1\n|; Also did a little manual editing on some of the arch/*/isa_traits.hh files and src/SConscript. diffstat: src/SConscript | 4 ++-- src/arch/alpha/mt.hh| 2 +- src/arch/alpha/pagetable.cc | 2 +- src/arch/alpha/tlb.cc | 2 +- src/arch/arm/faults.hh | 2 +- src/arch/arm/isa_traits.hh | 4 ++-- src/arch/arm/kernel_stats.hh| 4 ++-- src/arch/arm/nativetrace.cc | 2 +- src/arch/arm/nativetrace.hh | 2 +- src/arch/arm/tlb.hh | 2 +- src/arch/mips/dsp.hh| 2 +- src/arch/mips/faults.hh | 2 +- src/arch/mips/isa_traits.hh | 4 ++-- src/arch/mips/kernel_stats.hh | 4 ++-- src/arch/mips/linux/threadinfo.hh | 2 +- src/arch/power/faults.hh| 2 +- src/arch/power/insts/branch.hh | 2 +- src/arch/power/insts/condition.hh | 2 +- src/arch/power/insts/floating.hh| 2 +- src/arch/power/insts/integer.hh | 2 +- src/arch/power/insts/mem.hh | 2 +- src/arch/power/insts/misc.hh| 2 +- src/arch/power/insts/static_inst.hh | 2 +- src/arch/power/isa.hh | 2 +- src/arch/power/isa_traits.hh| 4 ++-- src/arch/power/locked_mem.hh| 2 +- src/arch/power/microcode_rom.hh | 2 +- src/arch/power/miscregs.hh | 2 +- src/arch/power/mmaped_ipr.hh| 2 +- src/arch/power/pagetable.cc | 2 +- src/arch/power/pagetable.hh | 2 +- src/arch/power/predecoder.hh| 2 +- src/arch/power/registers.hh | 2 +- src/arch/power/remote_gdb.hh| 2 +- src/arch/power/stacktrace.hh| 2 +- src/arch/power/tlb.hh | 2 +- src/arch/power/types.hh | 2 +- src/arch/power/utility.cc | 2 +- src/arch/power/utility.hh | 2 +- src/arch/power/vtophys.hh | 2 +- src/arch/sparc/faults.hh| 2 +- src/arch/sparc/kernel_stats.hh | 4 ++-- src/arch/sparc/nativetrace.cc | 2 +- src/arch/sparc/nativetrace.hh | 2 +- src/arch/sparc/tlb.cc | 2 +- src/arch/sparc/vtophys.cc | 2 +- src/arch/x86/cpuid.cc | 2 +- src/arch/x86/nativetrace.cc | 2 +- src/arch/x86/nativetrace.hh | 2 +- src/arch/x86/registers.hh | 2 +- src/arch/x86/tlb.cc | 2 +- src/arch/x86/utility.cc | 2 +- src/base/cprintf.cc | 2 +- src/base/cprintf.hh | 2 +- src/base/hashmap.hh | 2 +- src/base/inet.cc| 2 +- src/base/inet.hh| 2 +- src/base/mysql.cc | 2 +- src/base/mysql.hh | 2 +- src/base/statistics.cc | 2 +- src/base/statistics.hh | 2 +- src/base/stats/info.hh | 2 +- src/base/stats/mysql.cc | 2 +- src/base/stats/mysql.hh | 2 +- src/base/stats/mysql_run.hh | 2 +- src/base/stats/output.cc| 2 +- src/base/stats/output.hh| 2 +- src/base/stats/text.cc | 2 +- src/base/stats/text.hh | 2 +- src/base/stats/types.hh | 2 +- src/base/stats/visit.cc | 2 +- src/base/stats/visit.hh | 2 +- src/base/stl_helpers.hh | 4 ++-- src/base/trace.cc | 2 +- src/base/trace.hh | 2 +- src/base/varargs.hh | 2 +- src/cpu/exetrace.cc | 2 +- src/cpu/exetrace.hh | 2 +- src/cpu/inorder/inorder_trace.cc| 2 +- src/cpu/inorder/inorder_trace.hh| 2 +- src/cpu/inteltrace.cc | 2 +- src/cpu/inteltrace.hh | 2 +- src/cpu/legiontrace.cc | 2 +- src/cpu/legiontrace.hh | 2 +- src/cpu/nativetrace.cc | 2 +- src/cpu/nativetrace.hh | 2 +- src/dev/copy_engine_defs.hh | 2 +- src/dev/i8254xGBe_defs.hh | 2 +- src/dev/sinic.cc| 2 +- src/dev/sinic.hh| 2 +- src/dev/sinicreg.hh | 4 ++-- src/dev/x86/cmos.hh | 2 +- src/dev/x86/i8042.hh| 2 +- src/dev/x86/i82094aa.hh | 2 +- src/dev/x86/i8237.hh| 2 +- src/dev/x86/i8254.hh| 2 +- src/dev/x86/i8259.hh| 2 +-
[m5-dev] changeset in m5: Delete unused files from src/base directory.
changeset 850d71d21135 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=850d71d21135
description:
Delete unused files from src/base directory.
diffstat:
src/base/SConscript |6 -
src/base/compression/base.hh | 71 --
src/base/compression/lzss_compression.cc | 173 ---
src/base/compression/lzss_compression.hh | 103
src/base/compression/null_compression.hh | 63 --
src/base/crc.cc | 105
src/base/crc.hh | 39 -
src/base/fifo_buffer.cc | 43 -
src/base/fifo_buffer.hh | 89 ---
src/base/hybrid_pred.cc | 276 ---
src/base/hybrid_pred.hh | 204
src/base/predictor.hh| 63 --
src/base/res_list.hh | 759 ---
src/base/sat_counter.cc | 271 ---
src/base/sat_counter.hh | 195 ---
15 files changed, 0 insertions(+), 2460 deletions(-)
diffs (truncated from 2545 to 300 lines):
diff -r a8fc35183c10 -r 850d71d21135 src/base/SConscript
--- a/src/base/SConscript Mon Jan 03 14:35:43 2011 -0800
+++ b/src/base/SConscript Mon Jan 03 14:35:45 2011 -0800
@@ -38,14 +38,11 @@
Source('callback.cc')
Source('circlebuf.cc')
Source('cprintf.cc')
-Source('crc.cc')
Source('debug.cc')
Source('fast_alloc.cc')
if env['USE_FENV']:
Source('fenv.c')
-Source('fifo_buffer.cc')
Source('hostinfo.cc')
-Source('hybrid_pred.cc')
Source('inet.cc')
Source('inifile.cc')
Source('intmath.cc')
@@ -58,7 +55,6 @@
Source('range.cc')
if env['TARGET_ISA'] != 'no':
Source('remote_gdb.cc')
-Source('sat_counter.cc')
Source('socket.cc')
Source('statistics.cc')
Source('str.cc')
@@ -66,8 +62,6 @@
Source('trace.cc')
Source('userinfo.cc')
-Source('compression/lzss_compression.cc')
-
Source('loader/aout_object.cc')
Source('loader/ecoff_object.cc')
Source('loader/elf_object.cc')
diff -r a8fc35183c10 -r 850d71d21135 src/base/compression/base.hh
--- a/src/base/compression/base.hh Mon Jan 03 14:35:43 2011 -0800
+++ /dev/null Thu Jan 01 00:00:00 1970 +
@@ -1,71 +0,0 @@
-/*
- * Copyright (c) 2003-2005 The Regents of The University of Michigan
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * AS IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * Authors: Erik Hallnor
- * Nathan Binkert
- */
-
-#ifndef __BASE_COMPRESSION_BASE_HH__
-#define __BASE_COMPRESSION_BASE_HH__
-
-/**
- * @file
- * This file defines a base (abstract virtual) compression algorithm object.
- */
-
-#include base/types.hh
-
-/**
- * Abstract virtual compression algorithm object.
- */
-class CompressionAlgorithm
-{
- public:
-virtual ~CompressionAlgorithm() {}
-
-/**
- * Uncompress the data, causes a fatal since no data should be compressed.
- * @param dest The output buffer.
- * @param src The compressed data.
- * @param size The number of bytes in src.
- *
- * @retval The size of the uncompressed data.
- */
-virtual int uncompress(uint8_t * dest, uint8_t *src, int size) = 0;
-
-/**
- * Compress the data, just returns the source data.
- * @param dest The output buffer.
- * @param src The data to be compressed.
- * @param size The number of bytes in src.
- *
- * @retval The size of the compressed data.
- */
-virtual int compress(uint8_t *dest, uint8_t *src, int size) = 0;
-};
-
-#endif //__BASE_COMPRESSION_BASE_HH__
[m5-dev] changeset in m5: Move sched_list.hh and timebuf.hh from src/base...
changeset 7338bc628489 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=7338bc628489
description:
Move sched_list.hh and timebuf.hh from src/base to src/cpu.
These files really aren't general enough to belong in src/base.
This patch doesn't reorder include lines, leaving them unsorted
in many cases, but Nate's magic script will fix that up shortly.
diffstat:
src/base/sched_list.hh| 180
src/base/timebuf.hh | 241 --
src/cpu/activity.cc |2 +-
src/cpu/activity.hh |2 +-
src/cpu/inorder/cpu.hh|2 +-
src/cpu/inorder/first_stage.hh|2 +-
src/cpu/inorder/pipeline_stage.hh |2 +-
src/cpu/o3/commit.hh |2 +-
src/cpu/o3/commit_impl.hh |2 +-
src/cpu/o3/cpu.hh |2 +-
src/cpu/o3/decode.hh |2 +-
src/cpu/o3/fetch.hh |2 +-
src/cpu/o3/fu_pool.hh |2 +-
src/cpu/o3/iew.hh |2 +-
src/cpu/o3/iew_impl.hh|2 +-
src/cpu/o3/inst_queue.hh |2 +-
src/cpu/o3/rename.hh |2 +-
src/cpu/ozone/back_end.hh |2 +-
src/cpu/ozone/cpu.hh |2 +-
src/cpu/ozone/front_end.hh|2 +-
src/cpu/ozone/inorder_back_end.hh |2 +-
src/cpu/ozone/inst_queue.hh |2 +-
src/cpu/ozone/lw_back_end.hh |2 +-
src/cpu/sched_list.hh | 180
src/cpu/timebuf.hh| 241 ++
25 files changed, 442 insertions(+), 442 deletions(-)
diffs (truncated from 1110 to 300 lines):
diff -r 850d71d21135 -r 7338bc628489 src/base/sched_list.hh
--- a/src/base/sched_list.hhMon Jan 03 14:35:45 2011 -0800
+++ /dev/null Thu Jan 01 00:00:00 1970 +
@@ -1,180 +0,0 @@
-/*
- * Copyright (c) 2002-2005 The Regents of The University of Michigan
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * AS IS AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * Authors: Steve Raasch
- */
-
-#ifndef SCHED_LIST_HH
-#define SCHED_LIST_HH
-
-#include list
-#include base/intmath.hh
-#include base/misc.hh
-
-
-// Any types you use this class for must be covered here...
-namespace {
-void ClearEntry(int i) { i = 0; };
-void ClearEntry(unsigned i) { i = 0; };
-void ClearEntry(double i) { i = 0; };
-template class T void ClearEntry(std::listT l) { l.clear(); };
-};
-
-
-//
-// this is a special list type that allows the user to insert elements at a
-// specified positive offset from the current element, but only allow them
-// be extracted from the current element
-//
-
-
-template class T
-class SchedList
-{
-T *data_array;
-unsigned position;
-unsigned size;
-unsigned mask;
-
- public:
-SchedList(unsigned size);
-SchedList(void);
-
-void init(unsigned size);
-
-T operator[](unsigned offset);
-
-void advance(void);
-
-void clear(void);
-};
-
-
-
-//
-// Constructor
-//
-templateclass T
-SchedListT::SchedList(unsigned _size)
-{
-size = _size;
-
-// size must be a power of two
-if (!isPowerOf2(size)) {
-panic(SchedList: size must be a power of two);
-}
-
-if (size 2) {
-panic(SchedList: you don't want a list that small);
-}
-
-// calculate the bit mask for the modulo operation
-mask = size - 1;
-
-data_array = new T[size];
-
-if (!data_array) {
-
Re: [m5-dev] Review Request: Ruby: Update MOESI CMP token protocol
On 2011-01-03 10:14:44, Steve Reinhardt wrote: src/mem/protocol/MOESI_CMP_token-L1cache.sm, line 972 http://reviews.m5sim.org/r/326/diff/2/?file=8135#file8135line972 These functions that take multiple pointers just so the called function can test which one is valid are pretty unwieldy and also seem to cause a lot of redundant testing. Can we factor out the check so we have a single test to set one valid entry_ptr variable, then use that in all these calls? I see there are some similar calls in MOESI_CMP_directory too, though not as many. Nilay Vaish wrote: You should check my comment in the MOESI Hammer request why we cannot set just one valid entry pointer. I am of the view that we should generate the arguments for getCacheEntry() internally in SLICC. Steve Reinhardt wrote: I understand that you can't universally assume that there's just one valid pointer... I'm not asking to go all the way back to that. But it appears to me that there are a lot of places (regions of code) where you know that only one of N pointers is valid, yet you have to repeatedly pass all N pointers to each low-level function to figure out which one to use. It would be both more efficient and easier to read if, in those regions, there was one function up front that identified and selected the one valid pointer (maybe sticking it in a local variable), then the low-level functions were all passed that one valid pointer. Auto-generating params in SLICC would solve the readability problem but not the efficiency problem. This is possible with a local variable. In fact I was thinking of letting SLICC generate such a local variable in each action. But then I decided I would let this issue be taken up during the review process. I can make this change. The problem is I am not sure if the currently proposed changes would be acceptable to some one (read Brad) who has used SLICC extensively. - Nilay --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/#review588 --- On 2010-12-31 17:33:04, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/326/ --- (Updated 2010-12-31 17:33:04) Review request for Default. Summary --- This request for reviewing the updates to implementation of the MOESI CMP token protocol. These updates have been carried out so as to conform with the changes made to CacheMemory and TBETable classes, and to SLICC. Diffs - src/mem/protocol/MOESI_CMP_token-L1cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-L2cache.sm UNKNOWN src/mem/protocol/MOESI_CMP_token-dir.sm UNKNOWN Diff: http://reviews.m5sim.org/r/326/diff Testing --- Changes have been tested using ruby_random_test.py for 1,000,000 loads and 20 different seeds for random number generator. Thanks, Nilay ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
[m5-dev] Review Request: scons: show sources and targets when building.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Diffs - SConstruct 7338bc628489 src/SConscript 7338bc628489 src/arch/SConscript 7338bc628489 src/arch/isa_parser.py 7338bc628489 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/#review596 --- Hi Nilay, First, I must say this is an impressive amount of work. You definitely got a lot done over holiday break. :) Overall, this set of patches is definitely close, but I want to see if we can take them a step forward. Also I have a few suggestions that may make things easier. Finally, I have a bunch of minor questions/suggestions on individual lines, but I’ll hold off on those until you can respond to my higher-level questions. The main thing I would like to see improved is not having to differentiate between “entry” and “entry_ptr” in the .sm files. Am I correct that the only functions in the .sm files that are passed an “entry_ptr” are “is_valid_ptr”, “getCacheEntry”, and “set_cache_entry”? If so, it seems that all three functions are generated with unique python code, either in an AST file or StateMachine.py. Therefore, could we just pass these functions “entry” and rely on the underneath python code to generate the correct references? This would make things more readable, “is_valid_ptr()” becomes “is_valid”, and it doesn’t require the slicc programmer to understand which functions take an entry pointer versus the entry itself. If we can’t make such a change, I worry about how much extra complexity this change pushes on the slicc programmer. Also another suggestion to make things more readable, please replace the name L1IcacheMemory_entry with L1I_entry. Do the same for L1D_entry and L2_entry. That will shorten many of your lines. So am I correct that hammer’s simultaneous usage of valid L1 and L2 cache entries in certain transitions is the only reason that within all actions, the getCacheEntry calls take multiple cache entries? If so, I think it would be fairly trivial to use a tbe entry as an intermediary between the L1 and L2 for those particular hammer transitions. That way only one cache entry is valid at any particular time, and we can simply use the variable cache_entry in the actions. That should clean things up a lot. By the way, once you check in these patches, the MESI_CMP_directory protocol will be deprecated, correct? If so, make sure you include a patch that removes it from the regression tester. Brad - Brad On 2011-01-03 14:18:13, Nilay Vaish wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/358/ --- (Updated 2011-01-03 14:18:13) Review request for Default. Summary --- The purpose of this patch is to change the way CacheMemory interfaces with coherence protocols. Currently, whenever a cache controller (defined in the protocol under consideration) needs to carry out any operation on a cache block, it looks up the tag hash map and figures out whether or not the block exists in the cache. In case it does exist, the operation is carried out (which requires another lookup). Over a single clock cycle, multiple such lookups take place as observed through profiling of different protocols. It was seen that the tag lookup takes anything from 10% to 20% of the simulation time. In order to reduce this time, this patch is being posted. The CacheMemory class now will have a function that will return pointer to a cache block entry, instead of a reference (though the function that returns the reference has been retained currently). Functions have been introduced for setting/unsetting a pointer and check its pointer. Similar changes have been carried out for Transaction Buffer entries as well. Note that changes are required to both SLICC and the protocol files. This patch carries out changes to SLICC and committing this patch alone, I believe, will ___break___ all the protocols. I am working on patching the protocols as well. This patch is being put to get feed back from other developers. Diffs - src/mem/protocol/RubySlicc_Types.sm UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.hh UNKNOWN src/mem/ruby/slicc_interface/AbstractCacheEntry.cc UNKNOWN src/mem/ruby/system/CacheMemory.hh UNKNOWN src/mem/ruby/system/CacheMemory.cc UNKNOWN src/mem/ruby/system/TBETable.hh UNKNOWN src/mem/slicc/ast/ActionDeclAST.py UNKNOWN src/mem/slicc/ast/FormalParamAST.py UNKNOWN src/mem/slicc/ast/FuncCallExprAST.py UNKNOWN src/mem/slicc/ast/InPortDeclAST.py UNKNOWN src/mem/slicc/ast/MethodCallExprAST.py UNKNOWN src/mem/slicc/ast/TypeDeclAST.py UNKNOWN src/mem/slicc/ast/__init__.py UNKNOWN src/mem/slicc/parser.py UNKNOWN src/mem/slicc/symbols/StateMachine.py UNKNOWN Diff: http://reviews.m5sim.org/r/358/diff Testing --- I have tested these
Re: [m5-dev] Review Request: scons: show sources and targets when building.
I like the original (Ali's version) better. When was it confusing? Perhaps there's a particular action that would be better the other way around. Maybe we should have graduated levels of verbosity for this stuff where, for example, 0 is Ali's version, 1 is this version, and 2 is the full command. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Testing quick regressions pass Diffs * SConstruct (7338bc628489) * src/SConscript (7338bc628489) * src/arch/SConscript (7338bc628489) * src/arch/isa_parser.py (7338bc628489) View Diff http://reviews.m5sim.org/r/366/diff/ ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev ___ m5-dev mailing list
Re: [m5-dev] Review Request: scons: show sources and targets when building.
--- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ --- (Updated 2011-01-03 16:10:27.987336) Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. Summary --- scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal: vptype_AddrRange_wrap.cc - vptype_AddrRange_wrap.do (...skipping...) [EMBED PY] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py - param_BaseSimpleCPU.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_BaseSimpleCPU.py.cc - param_BaseSimpleCPU.py.do [EMBED PY] ALPHA_SE/python/m5/internal: param_LiveProcess.py - param_LiveProcess.py.cc [ CXX] ALPHA_SE/python/m5/internal: param_LiveProcess.py.cc - param_LiveProcess.py.do [ TRACING] ALPHA_SE/base: - traceflags.py [EMBED PY] ALPHA_SE/base: traceflags.py - traceflags.py.cc [ CXX] ALPHA_SE/base: traceflags.py.cc - traceflags.py.do [ CXX] ALPHA_SE/base: date.cc - date.do [LINK] ALPHA_SE: - m5.debug Diffs (updated) - SConstruct 7338bc628489 src/SConscript 7338bc628489 src/arch/SConscript 7338bc628489 src/arch/isa_parser.py 7338bc628489 Diff: http://reviews.m5sim.org/r/366/diff Testing --- quick regressions pass Thanks, Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
I think this is the first time I've updated an existing diff via reviewboard, and I thought it would give me a chance to enter some comments on the website before publishing it like a new diff, but it didn't. so here are the comments that go with this diff: I had originally been more aggressive about common prefix extraction, but there were some glitches that made me go with the path-based approach... however, in the process of uploading this diff I thought of some fixes. So consider this second diff as an alternative and not necessarily a replacement. Example output for the new diff, corresponding to the previous example selection: [ CXX] ALPHA_SE/sim/main: .cc - .do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] - ALPHA_SE/base/traceflags.hh [ CXX] ALPHA_SE/python/swig/pyevent: .cc - .do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] - ALPHA_SE/arch/isa_traits.hh [ CXX] ALPHA_SE/python/swig/pyobject: .cc - .do [SWIG] ALPHA_SE/python/swig/core: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/swig/core_wrap: .cc - .do [SWIG] ALPHA_SE/python/swig/debug: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/swig/debug_wrap: .cc - .do ... [GENERATE] - ALPHA_SE/arch/vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] - ALPHA_SE/arch/types.hh [GENERATE] - ALPHA_SE/arch/registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha/: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base/remote_gdb: .cc - .do [ CXX] ALPHA_SE/base/socket: .cc - .do ... [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal/vptype_Process: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/m5/internal/vptype_Process_wrap: .cc - .do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal/vptype_AddrRange: .i - _wrap.cc, .py [ CXX] ALPHA_SE/python/m5/internal/vptype_AddrRange_wrap: .cc - .do ... [EMBED PY] ALPHA_SE/python/m5/internal/param_BaseSimpleCPU.py: - .cc [ CXX] ALPHA_SE/python/m5/internal/param_BaseSimpleCPU.py: .cc - .do [EMBED PY] ALPHA_SE/python/m5/internal/param_LiveProcess.py: - .cc [ CXX] ALPHA_SE/python/m5/internal/param_LiveProcess.py: .cc - .do [ TRACING] - ALPHA_SE/base/traceflags.py [EMBED PY] ALPHA_SE/base/traceflags.py: - .cc [ CXX] ALPHA_SE/base/traceflags.py: .cc - .do [ CXX] ALPHA_SE/base/date: .cc - .do [LINK] - ALPHA_SE/m5.debug ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
I don't recall all the details about what got me going on this, but I think a couple of the specific things I didn't like were (1) if you compile multiple binaries at the same time (e.g., m5.debug and m5.opt) the CXX lines print the source file which doesn't give you enough information to know which one is compiling and (2) for swig, it prints the foo_wrap.cc target file name, which takes some (admittedly little) inside knowledge to map back to the foo.i file if you care, and doesn't mention the generated .py file. Anyway, it was just enough annoyance to lead me to think that the problem was inconsistently printing the source sometimes and the target other times... one simple solution would be just to consistently print one or the other, but since it's not obvious which one is more useful, I thought why not print both. And here I am. I think this version would be helpful in addressing the newbies-don't-know-where-generated-files-come-from problem too, if they just bother to look at the output while it builds. What don't you like about these newer versions? Just that they're more verbose? Steve On Mon, Jan 3, 2011 at 3:45 PM, Gabe Black gbl...@eecs.umich.edu wrote: I like the original (Ali's version) better. When was it confusing? Perhaps there's a particular action that would be better the other way around. Maybe we should have graduated levels of verbosity for this stuff where, for example, 0 is Ali's version, 1 is this version, and 2 is the full command. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh [SO PARAM] PhysicalMemory - ALPHA_SE/params/PhysicalMemory.hh [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc [ CXX] ALPHA_SE/base: remote_gdb.cc - remote_gdb.do [ CXX] ALPHA_SE/base: socket.cc - socket.do (...skipping...) [SW PARAM] Process - ALPHA_SE/python/m5/internal/vptype_Process.i [BLDPARAM] Process - ALPHA_SE/python/m5/internal/param_Process.i [BLDPARAM] SimObject - ALPHA_SE/python/m5/internal/param_SimObject.i [BLDPARAM] System - ALPHA_SE/python/m5/internal/param_System.i [ENUMSWIG] MemoryMode - ALPHA_SE/python/m5/internal/enum_MemoryMode.i [BLDPARAM] PhysicalMemory - ALPHA_SE/python/m5/internal/param_PhysicalMemory.i [BLDPARAM] MemObject - ALPHA_SE/python/m5/internal/param_MemObject.i [SWIG] ALPHA_SE/python/m5/internal: vptype_Process.i - vptype_Process_wrap.cc, vptype_Process.py [ CXX] ALPHA_SE/python/m5/internal: vptype_Process_wrap.cc - vptype_Process_wrap.do [SW PARAM] AddrRange - ALPHA_SE/python/m5/internal/vptype_AddrRange.i [SWIG] ALPHA_SE/python/m5/internal: vptype_AddrRange.i - vptype_AddrRange_wrap.cc, vptype_AddrRange.py [ CXX] ALPHA_SE/python/m5/internal:
Re: [m5-dev] Review Request: scons: show sources and targets when building.
Yeah, primarily. In this new version there's enough text that it gets hard to pick out the nugget that lets you know what it's doing. We have some way to turn on the command lines again, don't we? I don't really remember off hand. Granted when things are compiling I usually just ignore it and check it every now and then to see if it finished or failed, but it was nice that it was relatively clean and sparse and would give you a quick idea of what it was chewing on. I also think every time we say if they just X we're forgetting that many people won't know or think to just X, so we shouldn't bend things around too far to accommodate that (not saying that's the case here, but still). For instance, Ali's links after warns, panics, etc. are a pretty good idea if people were to just follow them and put some info there, but I'd be willing to bet very few or even none of those links actually has anything. And actually speaking of which, we might want to get rid of those links. Gabe Steve Reinhardt wrote: I don't recall all the details about what got me going on this, but I think a couple of the specific things I didn't like were (1) if you compile multiple binaries at the same time (e.g., m5.debug and m5.opt) the CXX lines print the source file which doesn't give you enough information to know which one is compiling and (2) for swig, it prints the foo_wrap.cc target file name, which takes some (admittedly little) inside knowledge to map back to the foo.i file if you care, and doesn't mention the generated .py file. Anyway, it was just enough annoyance to lead me to think that the problem was inconsistently printing the source sometimes and the target other times... one simple solution would be just to consistently print one or the other, but since it's not obvious which one is more useful, I thought why not print both. And here I am. I think this version would be helpful in addressing the newbies-don't-know-where-generated-files-come-from problem too, if they just bother to look at the output while it builds. What don't you like about these newer versions? Just that they're more verbose? Steve On Mon, Jan 3, 2011 at 3:45 PM, Gabe Black gbl...@eecs.umich.edu mailto:gbl...@eecs.umich.edu wrote: I like the original (Ali's version) better. When was it confusing? Perhaps there's a particular action that would be better the other way around. Maybe we should have graduated levels of verbosity for this stuff where, for example, 0 is Ali's version, 1 is this version, and 2 is the full command. Gabe Steve Reinhardt wrote: This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/366/ Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan Binkert. By Steve Reinhardt. Description scons: show sources and targets when building. I like the brevity of Ali's recent change, but the ambiguity of sometimes showing the source and sometimes the target is a little confusing. This patch makes scons typically list all sources and all targets for each action, with the common path prefix factored out for brevity. It's a little more verbose now but also more informative. I'm not intending to add this to the commit message, but for review purposes, here's some example output: [ CXX] ALPHA_SE/sim: main.cc - main.do Defining FAST_ALLOC_DEBUG as 0 in build/ALPHA_SE/config/fast_alloc_debug.hh. Defining FAST_ALLOC_STATS as 0 in build/ALPHA_SE/config/fast_alloc_stats.hh. Defining NO_FAST_ALLOC as 0 in build/ALPHA_SE/config/no_fast_alloc.hh. [ TRACING] ALPHA_SE/base: - traceflags.hh [ CXX] ALPHA_SE/python/swig: pyevent.cc - pyevent.do Defining FULL_SYSTEM as 0 in build/ALPHA_SE/config/full_system.hh. [SO PARAM] MemObject - ALPHA_SE/params/MemObject.hh [SO PARAM] SimObject - ALPHA_SE/params/SimObject.hh [GENERATE] ALPHA_SE/arch: - isa_traits.hh [ CXX] ALPHA_SE/python/swig: pyobject.cc - pyobject.do [SWIG] ALPHA_SE/python/swig: core.i - core_wrap.cc, core.py [ CXX] ALPHA_SE/python/swig: core_wrap.cc - core_wrap.do [SWIG] ALPHA_SE/python/swig: debug.i - debug_wrap.cc, debug.py [ CXX] ALPHA_SE/python/swig: debug_wrap.cc - debug_wrap.do (...skipping...) [GENERATE] ALPHA_SE/arch: - vtophys.hh [ CFG ISA] alpha, arm, mips, no, power, sparc, x86 - ALPHA_SE/config/the_isa.hh [GENERATE] ALPHA_SE/arch: - types.hh [GENERATE] ALPHA_SE/arch: - registers.hh [GENERATE] static_inst_exec_sigs.hh: AtomicSimpleCPU, InOrderCPU, O3CPU, TimingSimpleCPU [EN PARAM] MemoryMode - ALPHA_SE/enums/MemoryMode.hh [SO PARAM] System - ALPHA_SE/params/System.hh [EN PARAM] OpClass - ALPHA_SE/enums/OpClass.hh
Re: [m5-dev] Review Request: scons: show sources and targets when building.
I had originally been more aggressive about common prefix extraction, but there were some glitches that made me go with the path-based approach... however, in the process of uploading this diff I thought of some fixes. So consider this second diff as an alternative and not necessarily a replacement. Example output for the new diff, corresponding to the previous example selection: I love this and I've been wanting it for a while for your reason #1, also, there are some ambiguous things where the same file is printed because one is a TARGET and another thing uses a SOURCE (that always bothered me.) [ CXX] ALPHA_SE/sim/main: .cc - .do My only change would be to remove the colon space. It serves little purpose and makes cut-and-paste less useful. [ CXX] ALPHA_SE/sim/main.cc - .do Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Jan 3, 2011, at 10:21 PM, nathan binkert wrote: I had originally been more aggressive about common prefix extraction, but there were some glitches that made me go with the path-based approach... however, in the process of uploading this diff I thought of some fixes. So consider this second diff as an alternative and not necessarily a replacement. Example output for the new diff, corresponding to the previous example selection: I love this and I've been wanting it for a while for your reason #1, also, there are some ambiguous things where the same file is printed because one is a TARGET and another thing uses a SOURCE (that always bothered me.) [ CXX] ALPHA_SE/sim/main: .cc - .do My only change would be to remove the colon space. It serves little purpose and makes cut-and-paste less useful. [ CXX] ALPHA_SE/sim/main.cc - .do I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: RefCount: Add a unit test for reference counting pointers.
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/#review597
---
Ship it!
I have some suggestions, but overall, it looks good. It would be nice to at
least have the return value of the test indicate success or failure as it would
be nice to actually include these in the regressions in the future. Thanks!
src/unittest/refcnttest.cc
http://reviews.m5sim.org/r/365/#comment832
This diff looks overall correct, though the use of assert to do this is a
bit odd. Perhaps replace assert with something more like (though it might not
be very meaningful without a description of what you're testing to go with it):
#define test(X) do {
bool test = (X);
cprintf(%-7s (line %3d) %s\n, test ? SUCCESS : FAIL, __LINE__, #X);
if (!test) {
Debug::break();
failures++;
}
} while (0)
at the exit of main(), i'd print total failures and also do return
!!failures !!x is equivalent to x ? 1 : 0 or int(bool(x)), if you don't
like the shorthand.
- Nathan
On 2011-01-03 12:56:11, Gabe Black wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/365/
---
(Updated 2011-01-03 12:56:11)
Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and
Nathan Binkert.
Summary
---
RefCount: Add a unit test for reference counting pointers.
This test exercises each of the functions in the reference counting pointer
implementation individually (except get()) and verifies they have some
minimially expected behavior. It also checks that reference counted objects
are freed when their usage count goes to 0 in some basic situations,
specifically a pointer being set to NULL and a pointer being deleted.
Diffs
-
src/unittest/SConscript 3a790012d6ed
src/unittest/refcnttest.cc PRE-CREATION
Diff: http://reviews.m5sim.org/r/365/diff
Testing
---
Thanks,
Gabe
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
I love this and I've been wanting it for a while for your reason #1, also, there are some ambiguous things where the same file is printed because one is a TARGET and another thing uses a SOURCE (that always bothered me.) [ CXX] ALPHA_SE/sim/main: .cc - .do My only change would be to remove the colon space. It serves little purpose and makes cut-and-paste less useful. [ CXX] ALPHA_SE/sim/main.cc - .do I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. I frequently compile many different versions of the source, and I like to know if it's working on the .do or the .fo (so I can get an idea of how far things are along.) If Gabe and Ali don't like this, we could go so far as to do Gabe's idea of VERBOSE=1 or VERBOSE=2, though that seems like overkill to me. I personally really do like this diff and would hate to see it go. Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Mon, Jan 3, 2011 at 8:38 PM, nathan binkert n...@binkert.org wrote: I love this and I've been wanting it for a while for your reason #1, also, there are some ambiguous things where the same file is printed because one is a TARGET and another thing uses a SOURCE (that always bothered me.) Glad it's not just me :-). [ CXX] ALPHA_SE/sim/main: .cc - .do My only change would be to remove the colon space. It serves little purpose and makes cut-and-paste less useful. [ CXX] ALPHA_SE/sim/main.cc - .do That's a decent idea... I didn't do it that way because it makes it potentially ambiguous about where the new extension gets spliced in, but in practice that's got to be rare, and having at least one file name you can cut and paste is a win (that's one thing I wasn't thrilled with about either of my patches). I'm ok with it, but I still think the cases where there is a problem are few are far between. Anyone who really has a problem should turn on the verbose printing. There's a huge difference in verbosity and readability between this and verbose... if Ali's patch was 0 and this is a 1, the old verbose is like 18. Not to mention that this makes it clear what the sources and targets are, which is hard to do when switching around between different command lines for different tools. I frequently compile many different versions of the source, and I like to know if it's working on the .do or the .fo (so I can get an idea of how far things are along.) If Gabe and Ali don't like this, we could go so far as to do Gabe's idea of VERBOSE=1 or VERBOSE=2, though that seems like overkill to me. I personally really do like this diff and would hate to see it go. With Nate's suggested change, for CXX lines we're really just tacking on the ' - .o' at the end... the initial part of the line is unchanged, and the number of extra chars per line is 6 or 7. If you guys really insist, it probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Jan 3, 2011, at 10:57 PM, Steve Reinhardt wrote: With Nate's suggested change, for CXX lines we're really just tacking on the ' - .o' at the end... the initial part of the line is unchanged, and the number of extra chars per line is 6 or 7. If you guys really insist, it probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. I'm fine with the change. Ali ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
On Mon, Jan 3, 2011 at 8:57 PM, Steve Reinhardt ste...@gmail.com wrote: [ CXX] ALPHA_SE/sim/main: .cc - .do My only change would be to remove the colon space. It serves little purpose and makes cut-and-paste less useful. [ CXX] ALPHA_SE/sim/main.cc - .do That's a decent idea... I didn't do it that way because it makes it potentially ambiguous about where the new extension gets spliced in, but in practice that's got to be rare, and having at least one file name you can cut and paste is a win (that's one thing I wasn't thrilled with about either of my patches). So far I've found two oddities with a straightforward implementation of this approach: (1) It breaks down when the target is a prefix of the source: [ STRIP] ALPHA_SE/m5.fast.unstripped - instead of [ STRIP] ALPHA_SE/m5.fast: .unstripped - (2) It gives the wrong impression when targets are in a parent directory relative to the source: [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc instead of [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc I don't see these as deal-breakers, and I'm fine with leaving them as they are unless someone has a reasonable idea about how to address them, but I thought I'd point out that the ambiguities aren't purely theoretical. Steve ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
Ali Saidi wrote: On Jan 3, 2011, at 10:57 PM, Steve Reinhardt wrote: With Nate's suggested change, for CXX lines we're really just tacking on the ' - .o' at the end... the initial part of the line is unchanged, and the number of extra chars per line is 6 or 7. If you guys really insist, it probably wouldn't be too hard to make this controlled by an option, but I agree that it seems like overkill. I'm fine with the change. Ali Yeah, I'm probably ok with it too. I like the old version a bit better, but it's not a big deal if you guys really like the new way. Gabe ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
Re: [m5-dev] Review Request: scons: show sources and targets when building.
So far I've found two oddities with a straightforward implementation of this approach: (1) It breaks down when the target is a prefix of the source: [ STRIP] ALPHA_SE/m5.fast.unstripped - instead of [ STRIP] ALPHA_SE/m5.fast: .unstripped - (2) It gives the wrong impression when targets are in a parent directory relative to the source: [ISA DESC] ALPHA_SE/arch/alpha/isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc instead of [ISA DESC] ALPHA_SE/arch/alpha: isa/main.isa - decoder.cc, decoder.hh, max_inst_regs.hh, atomic_simple_cpu_exec.cc, inorder_cpu_exec.cc, o3_cpu_exec.cc, timing_simple_cpu_exec.cc I don't see these as deal-breakers, and I'm fine with leaving them as they are unless someone has a reasonable idea about how to address them, but I thought I'd point out that the ambiguities aren't purely theoretical. hmmm I guess I still like the ability to cut and paste, but the ambiguity is annoying (and there are bound to be others. I'll let you decide. For #2, you could do ../decoder.cc, for #1, if you want, you could detect that the RHS is empty and do the whole basename. These ideas might be overkill. (Don't you love how a simple idea always gets more complicated?) Nate ___ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
