[LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor
jiwang at tilera.com
Fri Mar 1 17:19:07 PST 2013
On 03/02/2013 04:50 AM, Dmitri Gribenko wrote:
> You also need tests for Clang bits, too.
> Mechanical issues:
> +/// getTileRegisterNumbering - Given the enum value for some register,
> +/// return the number that it corresponds to.
> Please don't duplicate function and class name in comments. Existing
> code does this, but current style guidelines advise not to.
> + BuildMI(MBB, I, I->getDebugLoc(), TII->get(Tile::ADD)
> + ,I->getOperand(0).getReg())
> + .addReg(Tile::ZERO)
> + .addReg(src_sp);
> +unsigned TileInstrInfo::
> +isLoadFromStackSlot(const MachineInstr *MI, int &FrameIndex) const
> You have some weird formatting here and there. You could try
> clang-format to pretty-print your code automatically.
> The patch is also missing documentation bits. At the very least, a
> paragraph for ReleaseNotes.
> + // save lr to caller's stack reserve slot 0
> Comments should start with a capital letter and end with a full stop.
thanks for your time to review, I will fix these things according
to llvm coding style doc.
More information about the llvm-dev