> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote:
> > src/mem/ruby/system/Sequencer.py, line 77
> > <http://reviews.gem5.org/r/3189/diff/1/?file=51146#file51146line77>
> >
> >     Why is this default initialized to 16?
> 
> Brad Beckmann wrote:
>     Thank you for pointing this out.  We will remove the default setting of 
> 16.

Acutally we need the default setting for protocols that do not need or set the 
coreid parameter.  I modified the patch to set the default to 99 and I added a 
comment to make it clear that is a dummy value that should be overwritten.


> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote:
> > src/mem/ruby/slicc_interface/RubySlicc_ComponentMapping.hh, line 128
> > <http://reviews.gem5.org/r/3189/diff/1/?file=51133#file51133line128>
> >
> >     This function is functionally identical to createMachineID. Why add a 
> > separate function?

The function MachineTypeAndNodeIDToMachineID actually predates the function 
createMachineID.  The fact that this function was separately introduced by two 
different set of folks highlights why we need to get this patch checked in 
soon.  I have updated the patch to use createMachineID.


> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote:
> > src/mem/ruby/structures/CacheMemory.hh, line 187
> > <http://reviews.gem5.org/r/3189/diff/1/?file=51135#file51135line187>
> >
> >     Couple things:
> >     1) I see that initializing variables like this in the class declaration 
> > is new C++11 functionality. This kind of initialization is inconsistent 
> > with all other class member declarations in gem5, though, and this 
> > particular initialization doesn't appear to be necessary (see below).
> >     
> >     2) The m_block_size variable only appears to be used in 
> > CacheMemory::init(). Do we need to add it, or can it just be a local 
> > variable in init()?
> 
> Brad Beckmann wrote:
>     Thank you for pointing this out.  I did not write this code, but I'll 
> make sure that this is necessary.  Perhaps it was necessary to fix a problem 
> flagged by the compiler???  Also I'll figure out that Brandon/Nilay's 
> multi-instance ruby code fixes this problem.

I have removed the C++11 initialization.  I will not modify the rest of this 
patch because this code will be fixed in the multi-instance Ruby patches.  
Those patches are coming soon.


> On Nov. 13, 2015, 11:03 p.m., Joel Hestness wrote:
> > src/mem/ruby/structures/CacheMemory.cc, line 76
> > <http://reviews.gem5.org/r/3189/diff/1/?file=51136#file51136line76>
> >
> >     This looks like it could result in bugs. m_block_size is initialized to 
> > -1 in CacheMemory.hh, and possibly to sizes other than 
> > RubySystem::getBlockSizeBytes() in the constructor. Seems like it could 
> > either be incorrectly set and not 0 at the time init() is called (e.g. 2), 
> > or the initialization to -1 is unnecessary.
> >     
> >     1) Why not require that it be set when a CacheMemory is instantiated in 
> > Python?
> >     2) or, why not remove the initialization in the class declaration?
> 
> Brad Beckmann wrote:
>     I'll get back to you.

The multi-instance ruby patches fix/remove this code.  There is no need to fix 
it here.


- Brad


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


On Dec. 23, 2015, 5:11 p.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3189/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2015, 5:11 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> Changeset 11281:ed03615555f1
> ---------------------------
> gpu: AMD's baseline GPU model
> 
> This patch includes the entire baseline GPU model from AMD.  The patch needs
> to be checked in as one changeset, but let us know if you would prefer it to
> be split for reviewing purposes.
> 
> 
> Diffs
> -----
> 
>   src/gpu/global_memory_pipeline.hh PRE-CREATION 
>   src/gpu/global_memory_pipeline.cc PRE-CREATION 
>   src/gpu/gpu_dyn_inst.hh PRE-CREATION 
>   src/gpu/gpu_dyn_inst.cc PRE-CREATION 
>   src/gpu/gpu_exec_context.hh PRE-CREATION 
>   src/gpu/gpu_exec_context.cc PRE-CREATION 
>   src/gpu/gpu_static_inst.hh PRE-CREATION 
>   src/gpu/gpu_static_inst.cc PRE-CREATION 
>   src/gpu/gpu_tlb.hh PRE-CREATION 
>   src/gpu/gpu_tlb.cc PRE-CREATION 
>   src/gpu/hsa_code.hh PRE-CREATION 
>   src/gpu/hsa_kernel_info.hh PRE-CREATION 
>   src/gpu/hsa_object.hh PRE-CREATION 
>   src/gpu/hsa_object.cc PRE-CREATION 
>   src/gpu/code_enums.hh PRE-CREATION 
>   src/gpu/compute_unit.hh PRE-CREATION 
>   src/gpu/compute_unit.cc PRE-CREATION 
>   src/gpu/condition_register_state.hh PRE-CREATION 
>   src/gpu/condition_register_state.cc PRE-CREATION 
>   src/gpu/dispatcher.hh PRE-CREATION 
>   src/gpu/dispatcher.cc PRE-CREATION 
>   src/gpu/exec_stage.hh PRE-CREATION 
>   src/gpu/exec_stage.cc PRE-CREATION 
>   src/gpu/fetch_stage.hh PRE-CREATION 
>   src/gpu/fetch_stage.cc PRE-CREATION 
>   src/gpu/fetch_unit.hh PRE-CREATION 
>   src/gpu/fetch_unit.cc PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base.slicc PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-msg.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-probeFilter.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-Region-dir.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-Region-msg.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-RegionBuffer.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-RegionDir.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-dir.sm PRE-CREATION 
>   src/mem/protocol/GPU_VIPER-TCP.sm PRE-CREATION 
>   src/mem/protocol/GPU_VIPER.slicc PRE-CREATION 
>   src/mem/protocol/GPU_VIPER_Baseline.slicc PRE-CREATION 
>   src/mem/protocol/GPU_VIPER_Region-TCC.sm PRE-CREATION 
>   src/mem/protocol/GPU_VIPER_Region.slicc PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-CorePair.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-L3cache.sm PRE-CREATION 
>   src/mem/protocol/MOESI_AMD_Base-Region-CorePair.sm PRE-CREATION 
>   src/gpu/hsail_code.hh PRE-CREATION 
>   src/gpu/hsail_code.cc PRE-CREATION 
>   src/gpu/kernel_cfg.hh PRE-CREATION 
>   src/gpu/kernel_cfg.cc PRE-CREATION 
>   src/gpu/lds_state.hh PRE-CREATION 
>   src/gpu/lds_state.cc PRE-CREATION 
>   src/gpu/local_memory_pipeline.hh PRE-CREATION 
>   src/gpu/local_memory_pipeline.cc PRE-CREATION 
>   src/gpu/misc.hh PRE-CREATION 
>   src/gpu/ndrange.hh PRE-CREATION 
>   src/gpu/of_scheduling_policy.hh PRE-CREATION 
>   src/gpu/of_scheduling_policy.cc PRE-CREATION 
>   src/gpu/pool_manager.hh PRE-CREATION 
>   src/gpu/pool_manager.cc PRE-CREATION 
>   src/gpu/qstruct.hh PRE-CREATION 
>   src/gpu/rr_scheduling_policy.hh PRE-CREATION 
>   src/gpu/rr_scheduling_policy.cc PRE-CREATION 
>   src/gpu/schedule_stage.hh PRE-CREATION 
>   src/gpu/schedule_stage.cc PRE-CREATION 
>   src/gpu/scheduler.hh PRE-CREATION 
>   src/gpu/scheduler.cc PRE-CREATION 
>   src/gpu/scheduling_policy.hh PRE-CREATION 
>   src/gpu/scoreboard_check_stage.hh PRE-CREATION 
>   src/gpu/scoreboard_check_stage.cc PRE-CREATION 
>   src/gpu/shader.hh PRE-CREATION 
>   src/gpu/shader.cc PRE-CREATION 
>   src/gpu/simple_pool_manager.hh PRE-CREATION 
>   src/gpu/simple_pool_manager.cc PRE-CREATION 
>   src/gpu/tlb_coalescer.hh PRE-CREATION 
>   src/gpu/tlb_coalescer.cc PRE-CREATION 
>   src/gpu/vector_register_file.hh PRE-CREATION 
>   src/gpu/vector_register_file.cc PRE-CREATION 
>   src/gpu/vector_register_state.hh PRE-CREATION 
>   src/gpu/vector_register_state.cc PRE-CREATION 
>   src/gpu/wavefront.hh PRE-CREATION 
>   src/gpu/wavefront.cc PRE-CREATION 
>   src/mem/protocol/GPU_RfO-SQC.sm PRE-CREATION 
>   src/mem/protocol/GPU_RfO-TCC.sm PRE-CREATION 
>   src/mem/protocol/GPU_RfO-TCCdir.sm PRE-CREATION 
>   src/mem/protocol/GPU_RfO-TCP.sm PRE-CREATION 
>   src/mem/protocol/GPU_RfO.slicc PRE-CREATION 
>   src/mem/protocol/GPU_VIPER-SQC.sm PRE-CREATION 
>   src/mem/protocol/GPU_VIPER-TCC.sm PRE-CREATION 
>   src/gpu/arch/hsail/operand.cc PRE-CREATION 
>   src/gpu/arch/hsail/types.hh PRE-CREATION 
>   src/gpu/brig_object.hh PRE-CREATION 
>   src/gpu/brig_object.cc PRE-CREATION 
>   src/gpu/cl_driver.hh PRE-CREATION 
>   src/gpu/cl_driver.cc PRE-CREATION 
>   src/gpu/cl_event.hh PRE-CREATION 
>   src/gpu/arch/hsail/insts/gpu_static_inst.cc PRE-CREATION 
>   src/gpu/arch/hsail/insts/main.cc PRE-CREATION 
>   src/gpu/arch/hsail/insts/mem.hh PRE-CREATION 
>   src/gpu/arch/hsail/insts/mem.cc PRE-CREATION 
>   src/gpu/arch/hsail/insts/mem_impl.hh PRE-CREATION 
>   src/gpu/arch/hsail/insts/pseudo_inst.cc PRE-CREATION 
>   src/gpu/arch/hsail/operand.hh PRE-CREATION 
>   src/gpu/arch/hsail/generic_types.cc PRE-CREATION 
>   src/gpu/arch/hsail/insts/branch.hh PRE-CREATION 
>   src/gpu/arch/hsail/insts/branch.cc PRE-CREATION 
>   src/gpu/arch/hsail/insts/decl.hh PRE-CREATION 
>   src/gpu/arch/hsail/insts/gpu_static_inst.hh PRE-CREATION 
>   configs/ruby/GPU_RfO.py PRE-CREATION 
>   configs/ruby/GPU_VIPER.py PRE-CREATION 
>   configs/ruby/GPU_VIPER_Baseline.py PRE-CREATION 
>   configs/ruby/GPU_VIPER_Region.py PRE-CREATION 
>   configs/ruby/MOESI_AMD_Base.py PRE-CREATION 
>   configs/ruby/MemCntrlConstructor.py PRE-CREATION 
>   src/SConscript d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/gpu/GPU.py PRE-CREATION 
>   src/gpu/LdsState.py PRE-CREATION 
>   src/gpu/SConscript PRE-CREATION 
>   src/gpu/X86GPUTLB.py PRE-CREATION 
>   src/gpu/arch/SConscript PRE-CREATION 
>   src/gpu/arch/hsail/Brig.h PRE-CREATION 
>   src/gpu/arch/hsail/Brig_new.hpp PRE-CREATION 
>   src/gpu/arch/hsail/SConscript PRE-CREATION 
>   src/gpu/arch/hsail/SConsopts PRE-CREATION 
>   src/gpu/arch/hsail/decoder.hh PRE-CREATION 
>   src/gpu/arch/hsail/gen.py PRE-CREATION 
>   src/gpu/arch/hsail/generic_types.hh PRE-CREATION 
>   configs/common/GPUTLBConfig.py PRE-CREATION 
>   configs/common/GPUTLBOptions.py PRE-CREATION 
>   configs/example/apu_se.py PRE-CREATION 
>   configs/example/ruby_gpu_random_test.py PRE-CREATION 
>   configs/ruby/AMD_Base_Constructor.py PRE-CREATION 
>   SConstruct d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   build_opts/X86_MOESI_AMD_Base PRE-CREATION 
>   build_opts/gpu_RfO PRE-CREATION 
>   build_opts/gpu_VIPER PRE-CREATION 
>   build_opts/gpu_VIPER_Baseline PRE-CREATION 
>   build_opts/gpu_VIPER_Region PRE-CREATION 
>   src/mem/ruby/structures/RubyCache.py 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/SConscript d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/profiler/Profiler.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/slicc_interface/AbstractCacheEntry.hh 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/slicc_interface/AbstractController.hh 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/slicc_interface/AbstractController.cc 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/slicc_interface/RubySlicc_ComponentMapping.hh 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/structures/CacheMemory.hh 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/structures/CacheMemory.cc 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/RubySlicc_Types.sm 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/SConsopts d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/RubySlicc_Exports.sm 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/protocol/RubySlicc_ComponentMapping.sm 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/GPUCoalescer.hh PRE-CREATION 
>   src/mem/ruby/system/GPUCoalescer.cc PRE-CREATION 
>   src/mem/slicc/symbols/StateMachine.py 
> d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/WeightedLRUReplacementPolicy.py PRE-CREATION 
>   src/mem/ruby/system/WeightedLRUPolicy.cc PRE-CREATION 
>   src/mem/ruby/system/WeightedLRUPolicy.hh PRE-CREATION 
>   src/mem/ruby/system/VIPERCoalescer.py PRE-CREATION 
>   src/mem/ruby/system/VIPERCoalescer.cc PRE-CREATION 
>   src/mem/ruby/system/VIPERCoalescer.hh PRE-CREATION 
>   src/mem/ruby/system/Sequencer.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/Sequencer.py d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/Sequencer.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/RubyPort.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/RubyPort.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/RubySystem.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/SConscript d9a0136ab8cc4b3cf4821d064140b857e60db0dd 
>   src/mem/ruby/system/GPUCoalescer.py PRE-CREATION 
> 
> Diff: http://reviews.gem5.org/r/3189/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tony Gutierrez
> 
>

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

Reply via email to