[llvm-commits] RFC: R600/SI backend v3

Tom Stellard tstellar at gmail.com
Mon May 14 10:24:45 PDT 2012


Hi,

Here is an updated version of the R600/SI backend.  You can
also find these patches in the r600-review-v3 branch here:
http://cgit.freedesktop.org/~tstellar/llvm/ The changes from the previous
version are listed below.

On Mon, May 07, 2012 at 10:52:43AM -0700, Jakob Stoklund Olesen wrote:
> 
> 
> It doesn't look like you are finished preparing this for submission. Besides the style issues you mention, most of the comments I found look like "TODO: Add full description".
>
> I, for one, find it tedious to read thousands of uncommented source lines. That might explain the lack of responses.
> 

These TODOs have been replaced by comments and more comments have been
added throughout the code.
 
> 
> This isn't the way to go:
> 
> +// We are piggybacking on the CommentFlag enum in MachineInstr.h to
> +// set bits in AsmPrinterFlags of the MachineInstruction. We will
> +// start at bit 16 and allocate down while LLVM will start at bit
> +// 1 and allocate up.
>
> We've had that discussion with Micah before.
> 

The ASMPrinter was previously removed from the backend since it is not
needed for R600 codegen.  This comment is stale and has been removed.
 
> 
> Is AMDILUtilityFunctions.h dumping a bunch of functions in the global namespace? That's not a good idea. Same thing for the llvm namespace.
> 

AMDILUtilityFunctions.cpp has been removed.  All of its functions were
either unused or only used in one place.  The only code that remains
in AMDILUtilityFunctions.h are some macro definitions.  Also, global
functions from AMDGPUUtil.h have been moved out of the llvm namespace.

> 
> You still have some old target hooks, that aren't used any more. (isMoveInstr, copyRegToReg, probably others).
> 
> 

These have been removed.

> I would suggest that you go over the code and make sure it is properly documented. Also remove dead code and fix the style issues while you are there.
> 

In addition to the changes above I went through and consolidated a
bunch of the code and removed one of the larger auto-generated files.
This version also contains bugs fixes and additional features, like
better support for texture instructions and integer operations.

Looking forward to your comments.

Thanks,
Tom Stellard




More information about the llvm-commits mailing list