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

[Jakob Stoklund Olesen]

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.

/jakob