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

Dmitri Gribenko gribozavr at gmail.com
Thu Nov 29 08:28:15 PST 2012


On Tue, Nov 27, 2012 at 12:37 AM, Tom Stellard <tom at stellard.net> wrote:
> Assuming I caught all the coding style mistakes you pointed out, what
> else needs to be done to be able to commit this code?

Hello Tom,

There are still a few classes of coding style issues...

+  /// CreateLiveInRegister - Helper function that adds Reg to the LiveIn list
+  /// of the DAG's MachineFunction.  This returns a Register SDNode
representing
+  /// Reg.
+  SDValue CreateLiveInRegister(SelectionDAG &DAG, const
TargetRegisterClass *RC,
+                                                  unsigned Reg, EVT VT) const;

Please don't duplicate function or class name inside the comment.
Please also don't duplicate the comment in .cpp file.  Old code is
written that way, but current coding style advises not to duplicate.

This link may be helpful:
<http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments>.

+#ifndef _AMDIL7XXDEVICEIMPL_H_
+#define _AMDIL7XXDEVICEIMPL_H_

Identifiers starting with underscore and a capital letter are reserved.

+  CFGTraits::insertCondBranchBefore(branchInstrPos,
+                                    CFGTraits::getBranchNzeroOpcode(oldOpcode),
+                                    passRep,
+»······»·······»·······»·······»·······»·······»·······»·······»·······branchDL);

No tabs, please.

+AMDGPUCFGStructurizer::AMDGPUCFGStructurizer(char &pid, TargetMachine &tm
+                                          )
+: MachineFunctionPass(pid), TM(tm), TII(tm.getInstrInfo()),
+  TRI(static_cast<const AMDGPURegisterInfo *>(tm.getRegisterInfo())
+  ) {
+}

Both right parentheses should be on previous lines.

+};   //end of class AMDGPUCFGPrepare

In general I don't see "end of class" and "end of function" comments
in LLVM, just namespaces.

+// The AMDGPUCypressDevice is similiar to the AMDGPUEvergreenDevice,
except it has
+// support for double precision operations. This device is used to represent
+// both the Cypress and Hemlock cards, which are commercially known as HD58XX
+// and HD59XX cards.
+class AMDGPUCypressDevice : public AMDGPUEvergreenDevice {

Lots of comments should have three slashes (to become documentation comments).

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/




More information about the llvm-dev mailing list