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

[Tom Stellard]

On Mon, May 07, 2012 at 10:52:43AM -0700, Jakob Stoklund Olesen wrote:
> 
> On May 7, 2012, at 6:17 AM, Tom Stellard <thomas.stellard at amd.com> wrote:
> 
> > Should I be interpreting
> > the lack of responses as approval
> 
> No.
> 
> 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.
> 
> 
> 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.
> 
> 
> Is AMDILUtilityFunctions.h dumping a bunch of functions in the global namespace? That's not a good idea. Same thing for the llvm namespace.
> 
> 
> You still have some old target hooks, that aren't used any more. (isMoveInstr, copyRegToReg, probably others).
> 
> 
> 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.
> 

Hi Jakob,

Thanks for taking a look at this.  I'll make these changes and resubmit
the patches.

-Tom

> /jakob
> 
>