----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3189/#review7897 -----------------------------------------------------------
SConstruct (line 1244) <http://reviews.gem5.org/r/3189/#comment6847> This makes it look like the two are not orthogonal, could you explain/comment? configs/common/GPUTLBConfig.py (line 76) <http://reviews.gem5.org/r/3189/#comment6848> the location of the file, and the name of the functions make it seem quite generic, when in fact it is very specific and makes quite some assumptions would it be worth putting this is a specific dir and making the names prefixed with apu/compute-gpu/gpgpu or similar? configs/common/GPUTLBConfig.py (line 203) <http://reviews.gem5.org/r/3189/#comment6849> the division of functionality is very hard to figure out. there is a top level somewhere that assembles some parts of the system? could you explain the design here, and how this fits together with the rest of the config system? configs/common/GPUTLBOptions.py (line 42) <http://reviews.gem5.org/r/3189/#comment6850> I think we need some form of prefix, apu/compute-gpu/gpgpu etc. Most, if not all, existing options do not use camelCase, but instead small caps and dashes. This is also really exposing the issues we have with the current config system...but that's a different story. configs/common/GPUTLBOptions.py (line 75) <http://reviews.gem5.org/r/3189/#comment6851> Quite a few questions around units here. configs/example/apu_se.py (line 86) <http://reviews.gem5.org/r/3189/#comment6852> the naming seems quite inconsistent with a mix of camelCase and dash-separated-names configs/example/apu_se.py (line 289) <http://reviews.gem5.org/r/3189/#comment6853> any reason why minor is not here? Why not use CpuConfig? configs/example/apu_se.py (line 305) <http://reviews.gem5.org/r/3189/#comment6854> CPU vs command processor? What is the difference? configs/example/apu_se.py (line 494) <http://reviews.gem5.org/r/3189/#comment6855> What is this based on? configs/example/ruby_gpu_random_test.py (line 113) <http://reviews.gem5.org/r/3189/#comment6856> besides the protocol it is not clear why the normal ruby test script is not used, there seems to be a lot of overlap configs/ruby/GPU_RfO.py (line 80) <http://reviews.gem5.org/r/3189/#comment6857> there seems to be an awful lot of repetition of boilerplate code involved in these scripts, can the common parts not be broken out? configs/ruby/GPU_VIPER_Baseline.py (line 126) <http://reviews.gem5.org/r/3189/#comment6858> again...there is a lot of repetition these files really need some modularity Initial pass through the first page - Andreas Hansson On Jan. 13, 2016, 9:25 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3189/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2016, 9:25 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11282:118397102cfe > --------------------------- > gpu: AMD's baseline GPU model > > This patch includes the entire baseline GPU model from AMD. > > > Diffs > ----- > > src/gpu-compute/hsa_object.cc PRE-CREATION > src/gpu-compute/hsail_code.hh PRE-CREATION > src/gpu-compute/hsail_code.cc PRE-CREATION > src/gpu-compute/kernel_cfg.hh PRE-CREATION > src/gpu-compute/kernel_cfg.cc PRE-CREATION > src/gpu-compute/lds_state.hh PRE-CREATION > src/gpu-compute/lds_state.cc PRE-CREATION > src/gpu-compute/local_memory_pipeline.hh PRE-CREATION > src/gpu-compute/local_memory_pipeline.cc PRE-CREATION > src/gpu-compute/misc.hh PRE-CREATION > src/gpu-compute/ndrange.hh PRE-CREATION > src/gpu-compute/of_scheduling_policy.hh PRE-CREATION > src/gpu-compute/of_scheduling_policy.cc PRE-CREATION > src/gpu-compute/pool_manager.hh PRE-CREATION > src/gpu-compute/pool_manager.cc PRE-CREATION > src/gpu-compute/qstruct.hh PRE-CREATION > src/gpu-compute/rr_scheduling_policy.hh PRE-CREATION > src/gpu-compute/rr_scheduling_policy.cc PRE-CREATION > src/gpu-compute/schedule_stage.hh PRE-CREATION > src/gpu-compute/schedule_stage.cc PRE-CREATION > src/gpu-compute/scheduler.hh PRE-CREATION > src/gpu-compute/scheduler.cc PRE-CREATION > src/gpu-compute/scheduling_policy.hh PRE-CREATION > src/gpu-compute/scoreboard_check_stage.hh PRE-CREATION > src/gpu-compute/scoreboard_check_stage.cc PRE-CREATION > src/gpu-compute/shader.hh PRE-CREATION > src/gpu-compute/shader.cc PRE-CREATION > src/gpu-compute/simple_pool_manager.hh PRE-CREATION > src/gpu-compute/simple_pool_manager.cc PRE-CREATION > src/gpu-compute/tlb_coalescer.hh PRE-CREATION > src/gpu-compute/tlb_coalescer.cc PRE-CREATION > src/gpu-compute/vector_register_file.hh PRE-CREATION > src/gpu-compute/vector_register_file.cc PRE-CREATION > src/gpu-compute/vector_register_state.hh PRE-CREATION > src/gpu-compute/vector_register_state.cc PRE-CREATION > src/gpu-compute/wavefront.hh PRE-CREATION > src/gpu-compute/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/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/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/MOESI_AMD_Base-msg.sm PRE-CREATION > src/mem/protocol/MOESI_AMD_Base-probeFilter.sm PRE-CREATION > src/mem/protocol/MOESI_AMD_Base.slicc PRE-CREATION > src/mem/protocol/RubySlicc_ComponentMapping.sm > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/protocol/RubySlicc_Exports.sm > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/protocol/RubySlicc_Types.sm > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/protocol/SConsopts 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/ruby/structures/RubyCache.py > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/ruby/system/GPUCoalescer.hh PRE-CREATION > src/mem/ruby/system/GPUCoalescer.cc PRE-CREATION > src/mem/ruby/system/GPUCoalescer.py PRE-CREATION > 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/Sequencer.hh d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/ruby/system/Sequencer.cc d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/ruby/system/Sequencer.py d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/mem/ruby/system/VIPERCoalescer.hh PRE-CREATION > src/mem/ruby/system/VIPERCoalescer.cc PRE-CREATION > src/mem/ruby/system/VIPERCoalescer.py PRE-CREATION > src/mem/ruby/system/WeightedLRUPolicy.hh PRE-CREATION > src/mem/ruby/system/WeightedLRUPolicy.cc PRE-CREATION > src/mem/ruby/system/WeightedLRUReplacementPolicy.py PRE-CREATION > src/mem/slicc/symbols/StateMachine.py > d9a0136ab8cc4b3cf4821d064140b857e60db0dd > src/gpu-compute/fetch_stage.cc PRE-CREATION > src/gpu-compute/fetch_unit.hh PRE-CREATION > src/gpu-compute/fetch_unit.cc PRE-CREATION > src/gpu-compute/global_memory_pipeline.hh PRE-CREATION > src/gpu-compute/global_memory_pipeline.cc PRE-CREATION > src/gpu-compute/gpu_dyn_inst.hh PRE-CREATION > src/gpu-compute/gpu_dyn_inst.cc PRE-CREATION > src/gpu-compute/gpu_exec_context.hh PRE-CREATION > src/gpu-compute/gpu_exec_context.cc PRE-CREATION > src/gpu-compute/gpu_static_inst.hh PRE-CREATION > src/gpu-compute/gpu_static_inst.cc PRE-CREATION > src/gpu-compute/gpu_tlb.hh PRE-CREATION > src/gpu-compute/gpu_tlb.cc PRE-CREATION > src/gpu-compute/hsa_code.hh PRE-CREATION > src/gpu-compute/hsa_kernel_info.hh PRE-CREATION > src/gpu-compute/hsa_object.hh 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 > 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 > 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-compute/GPU.py PRE-CREATION > src/gpu-compute/LdsState.py PRE-CREATION > src/gpu-compute/SConscript PRE-CREATION > src/gpu-compute/X86GPUTLB.py PRE-CREATION > src/gpu-compute/arch/SConscript PRE-CREATION > src/gpu-compute/arch/hsail/Brig.h PRE-CREATION > src/gpu-compute/arch/hsail/Brig_new.hpp PRE-CREATION > src/gpu-compute/arch/hsail/SConscript PRE-CREATION > src/gpu-compute/arch/hsail/SConsopts PRE-CREATION > src/gpu-compute/arch/hsail/decoder.hh PRE-CREATION > src/gpu-compute/arch/hsail/gen.py PRE-CREATION > src/gpu-compute/arch/hsail/generic_types.hh PRE-CREATION > src/gpu-compute/arch/hsail/generic_types.cc PRE-CREATION > src/gpu-compute/arch/hsail/insts/branch.hh PRE-CREATION > src/gpu-compute/arch/hsail/insts/branch.cc PRE-CREATION > src/gpu-compute/arch/hsail/insts/decl.hh PRE-CREATION > src/gpu-compute/arch/hsail/insts/gpu_static_inst.hh PRE-CREATION > src/gpu-compute/arch/hsail/insts/gpu_static_inst.cc PRE-CREATION > src/gpu-compute/arch/hsail/insts/main.cc PRE-CREATION > src/gpu-compute/arch/hsail/insts/mem.hh PRE-CREATION > src/gpu-compute/arch/hsail/insts/mem.cc PRE-CREATION > src/gpu-compute/arch/hsail/insts/mem_impl.hh PRE-CREATION > src/gpu-compute/arch/hsail/insts/pseudo_inst.cc PRE-CREATION > src/gpu-compute/arch/hsail/operand.hh PRE-CREATION > src/gpu-compute/arch/hsail/operand.cc PRE-CREATION > src/gpu-compute/arch/hsail/types.hh PRE-CREATION > src/gpu-compute/brig_object.hh PRE-CREATION > src/gpu-compute/brig_object.cc PRE-CREATION > src/gpu-compute/cl_driver.hh PRE-CREATION > src/gpu-compute/cl_driver.cc PRE-CREATION > src/gpu-compute/cl_event.hh PRE-CREATION > src/gpu-compute/code_enums.hh PRE-CREATION > src/gpu-compute/compute_unit.hh PRE-CREATION > src/gpu-compute/compute_unit.cc PRE-CREATION > src/gpu-compute/condition_register_state.hh PRE-CREATION > src/gpu-compute/condition_register_state.cc PRE-CREATION > src/gpu-compute/dispatcher.hh PRE-CREATION > src/gpu-compute/dispatcher.cc PRE-CREATION > src/gpu-compute/exec_stage.hh PRE-CREATION > src/gpu-compute/exec_stage.cc PRE-CREATION > src/gpu-compute/fetch_stage.hh PRE-CREATION > > Diff: http://reviews.gem5.org/r/3189/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
