Steve,

I believe that the entirety of the license came from our legal department at 
some point. Probably it was what they came up with as a good 3-clause BSD 
license. It should be consistent throughout our code base as well.


Joe


________________________________
From: Steve Reinhardt <[email protected]> on behalf of Steve Reinhardt 
<[email protected]>
Sent: Friday, January 8, 2016 7:06 PM
To: Gross, Joe; Andreas Sandberg; Gross, Joe; Gutierrez, Anthony; Steve 
Reinhardt; Andreas Hansson; Beckmann, Brad; Default
Subject: Re: Review Request 3189: gpu: AMD's baseline GPU model

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


On November 10th, 2015, 11:51 a.m. PST, 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.

On January 7th, 2016, 4:26 p.m. PST, 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?

On January 7th, 2016, 4:31 p.m. PST, Tony Gutierrez wrote:

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

On January 7th, 2016, 5:03 p.m. PST, Andreas Hansson wrote:

I just picked up on the fact that it's different, and was not sure why. It 
should not be a problem, but if things can be uniform across the code base 
that's probably easier for everyone.

On January 8th, 2016, 8:44 a.m. PST, Brad Beckmann wrote:

There is nothing we can do about it.  We are going to check it in using the 
current license.  By the way, ARM and HP broke the uniformity of the licenses 
several years ago.  There is really nothing new here.

On January 8th, 2016, 9:13 a.m. PST, Andreas Hansson wrote:

That is fine. As I pointed out, I am merely observing (and enquiring about) the 
fact that AMD has previously used a different license wording and formatting. 
The ARM license header is consistent throughout the code base.

We had previously received AMD legal approval to contribute bug fixes and 
feature enhancements to M5/gem5 under the existing license.  Since the GPU 
model is a much more significant contribution of AMD IP, we had to go back to 
legal for further approvals, which led to them insisting on the new "For use 
for simulation and test purposes only" clause.  How we ended up with different 
formatting on the standard BSD part in the process is a mystery to me though.


- Steve


On December 23rd, 2015, 9:11 a.m. PST, Tony Gutierrez wrote:

Review request for Default.
By Tony Gutierrez.

Updated Dec. 23, 2015, 9:11 a.m.

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)

View Diff<http://reviews.gem5.org/r/3189/diff/>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to