> On Nov. 10, 2015, 11:51 a.m., Andreas Sandberg wrote:
> > This is indeed very impressive! However, as it is, this patch is basically 
> > the definition of unreviewable. Could you split it into a handful of 
> > different patches for review purposes? ReviewBoard makes all kinds of 
> > disconcerting creaking noises when I try to browse the massive (almost 
> > 2MiB!) patch. Could you split it into a set of different patches, at least 
> > for review purposes?
> > 
> > I'd suggest something along these lines:
> >     - Memory system stuff (Ruby)
> >     - GPU memory components (TLB/page table walkers etc.)
> >     - Configuration
> >     - Main build system (i.e., not the scons files in gpu/)
> >     - Pipeline
> >     - Object loader(s)
> >     - ISA
> > 
> > Quickly eyeing the patch, I noticed two things:
> >     - The new GPU ISA constant lives in the same header as the CPU ISA. 
> > Please put it in a separate header. This will make things easier down the 
> > line if we ever decide to reduce the set of files we need to recompile for 
> > different ISAs.
> >     - Some classes in gpu/ live in the X86ISA name space. This seems wrong.
> 
> Andreas Hansson wrote:
>     In addition to these concerns, there also seems to be a license header 
> format that does not appear anywhere else in the codebase. Is there a good 
> reason for this?

That is the license required by our legal team. It's BSD-style, so it shouldn't 
be an issue.


- Tony


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


On Dec. 23, 2015, 9:11 a.m., Tony Gutierrez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3189/
> -----------------------------------------------------------
> 
> (Updated Dec. 23, 2015, 9:11 a.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