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

[Villmow, Micah]

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Jakob Stoklund Olesen
> Sent: Monday, May 07, 2012 10:53 AM
> To: Stellard, Thomas
> Cc: llvm-commits at cs.uiuc.edu; Tom Stellard
> Subject: Re: [llvm-commits] RFC: R600/SI backend v2
> 
> 
> 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.
[Villmow, Micah] Yep, I'm working on a different solution, but it requires large amount of changes. 
> 
> 
> 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).
[Villmow, Micah] Thanks for the feedback, I'll look into this and remove them.
> 
> 
> 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.
> 
> /jakob
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits