----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3421/#review8175 -----------------------------------------------------------
Why did you choose to pass this parameter around to everything instead of keeping the canonical value with the RubySystem instance and changing the accessor to not be static (i.e., RubySystem::getBlockSizeBytes() to ruby_system->getBlockSizeBytes()). Most of these objects have a pointer to a RubySystem already. I really don't like having to change add this parameter to so many constructors, etc. The block size is inherent to the system, it shouldn't ever change (in a running system). Therefore, it should be a parameter at object creation (in the Python scripts), not a parameter every single time a function is called. Let me know if this isn't clear, and I can point out in the patch where I would like to see this parameter removed. However, I think moving towards ruby_system->getBlockSizeBytes() will fix most places. Also, there seems to be many place where this would need to be updated in the SLICC files (e.g., int l2_select_low_bit, default="RubySystem::getBlockSizeBits()";). I noted one below, but a quick grep reveals many others. Finally, as noted below, in cases where the entire object needs the value, you should get the default value from the parent using Parent.cache_line_size. This will reduce the possibility for accidentally missing one place where someone forgot to change it. src/gpu-compute/GPU.py (line 157) <http://reviews.gem5.org/r/3421/#comment7023> Get this from the a parent? (See Tags.py) # Get the block size from the parent (system) block_size = Param.Int(Parent.cache_line_size, "block size in bytes") src/gpu-compute/shader.cc (line 54) <http://reviews.gem5.org/r/3421/#comment7024> I could have missed this, but doesn't GPU.py need to be updated? src/mem/protocol/MOESI_CMP_token-L1cache.sm (line 200) <http://reviews.gem5.org/r/3421/#comment7030> Do all of these similar lines need to be update too? src/mem/ruby/slicc_interface/Controller.py (line 42) <http://reviews.gem5.org/r/3421/#comment7025> Same as above. Should use parent as default src/mem/ruby/structures/DirectoryMemory.py (line 39) <http://reviews.gem5.org/r/3421/#comment7026> Parent as default src/mem/ruby/structures/RubyCache.py (line 43) <http://reviews.gem5.org/r/3421/#comment7027> Ditto. src/mem/ruby/structures/RubyPrefetcher.py (line 51) <http://reviews.gem5.org/r/3421/#comment7028> Ditto. src/mem/ruby/system/Sequencer.py (line 51) <http://reviews.gem5.org/r/3421/#comment7029> Ditto. - Jason Lowe-Power On April 4, 2016, 11:40 p.m., Brandon Potter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3421/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 11:40 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11423:41c966b4e307 > --------------------------- > ruby: add a couple of members to ruby objects to remove statics > > The block_size_bits and block_size_bytes members are added to all of the > ruby objects which previously relied on their respective 'RubySystem::' > variants. The goal is to pass in the values through Python parameter lists > and inheritance rather than rely on global variables to pass values. > > This changeset does not modify the functions and methods to use the > the new members; that will happen in subsequent changesets with the goal of > providing some clarity to the new changes rather than a large indecipherable > changeset. > > > Diffs > ----- > > src/mem/ruby/filters/BulkBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/BulkBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/H3BloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/H3BloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/LSB_CountingBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/RubySlicc_Defines.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/AbstractBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/AbstractBloomFilter.cc PRE-CREATION > src/mem/ruby/filters/BlockBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/BlockBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/MultiGrainBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/MultiGrainBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/NonCountingBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/NonCountingBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/SConscript cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/network/Network.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/LSB_CountingBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/MultiBitSelBloomFilter.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/filters/MultiBitSelBloomFilter.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/Check.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/Check.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/CheckTable.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/CheckTable.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/RubyTester.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/RubyTester.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/cpu/testers/rubytest/RubyTester.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/gpu-compute/GPU.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/gpu-compute/fetch_unit.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/gpu-compute/fetch_unit.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/gpu-compute/shader.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/gpu-compute/shader.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_directory-L2cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-L1cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-L2cache.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/protocol/MOESI_CMP_token-dir.sm > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/Sequencer.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/Sequencer.py cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/VIPERCoalescer.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/profiler/Profiler.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/profiler/Profiler.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/slicc_interface/AbstractController.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/slicc_interface/AbstractController.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/slicc_interface/Controller.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/slicc_interface/RubyRequest.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/CacheMemory.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/CacheMemory.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/DirectoryMemory.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/PerfectCacheMemory.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/PersistentTable.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/PersistentTable.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/Prefetcher.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/Prefetcher.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/RubyCache.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/structures/RubyPrefetcher.py > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/CacheRecorder.hh > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/CacheRecorder.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/GPUCoalescer.cc > cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/RubyPort.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/RubyPort.cc cfad34a15729e1d5e096245f5a80ded6e2c379ca > src/mem/ruby/system/RubySystem.hh cfad34a15729e1d5e096245f5a80ded6e2c379ca > > Diff: http://reviews.gem5.org/r/3421/diff/ > > > Testing > ------- > > > Thanks, > > Brandon Potter > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
