[llvm-commits] RFC: Merge branches/R600 into TOT for 3.2 release

[Chris Lattner]

On Oct 30, 2012, at 11:35 AM, Tom Stellard <tom at stellard.net> wrote:
>> Hi Tom,
>> 
>> Time is running short, but this would be great.  The best place to start is to begin decomposing the mega-patch into individual pieces that makes sense.  Do you have changes outside Target/AMDGPU?  Those would be the be place to start.
> 
> Not counting test cases, new intrinsics, and configuration files,
> there are no changes outside of Target/AMDGPU.  I always submit patches
> for changes I make outside of Target/AMDGPU.  Isn't that how Open Source
> is supposed to work? ;)

Well yeah, but sometimes you still have to ask :).  Can the intrinsics be pulled into the target directory ala Targets/MBlaze/MBlazeIntrinsics.td or Targets/NVPTX/NVPTXIntrinsics.td?

> What's the best way to break up the patch?  In the past I have divided it up
> into 4 patches:
> 
> 1. Core Target files (*TargetMachine, *InstInfo, etc.)
> 2. Target specific passes
> 3. .td files
> 4. Configuration / Intrinsic additions
> 
> Would something like this work?

Any way that you can slices it to land it in stages would be preferred.  Another major challenge will be to find someone to review the patch.  You don't need detailed comments on every design aspect, but it is important to give you a basic "yeah, the structure looks fine and there aren't any obviously gratuitous code style violations" is the sort of review I'm looking for.  I'm not sure a great way to get this, but asking on llvmdev is probably a good start.

> 
>> Also, it would make more sense to name the directory lib/Target/R600 than AMDGPU (R600 isn't AMD's only GPU).  :)
>> 
> 
> The naming schemes used for GPUs is really confusing, so I'll try to
> explain:
> 
> The code in lib/Target/AMDGPU supports two different GPU architectures.
> The first is the R600 architecture (in the Open Source drivers we name
> the architecture after the first GPU family that uses it).  The
> R600 architecture covers 4 families of GPU: R600, R700, Evergreen,
> Cayman.
> 
> The second is the Graphics Core Next architecture, which we call
> Southern Islands, this covers the Southern Islands family and possibly
> future GPU families too.
> 
> While these two architectures are completely different when it comes to
> registers and ISA, there are some concepts that they share, so it is
> useful to be able to share code between them.  This is why they are both
> in the AMDGPU directory.
> 
> If you want I can split these into two separate directories R600, and
> SI, but then I would need to come up with a some other way to share code
> between them.

Yeah, that is really confusing.  :)  If they really are similar enough to share code, then I'd still rather that you come up with a name other than "AMDGPU" for this.  AMD will presumably continue to come out with GPUs 5 or 10 years from now and at SOME point there will be a major divergence.  I'd rather you give it an inaccurate name like "R600" than an inaccurate name like "AMDGPU".

-Chris