[cfe-dev] [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor
Jiong Wang
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.
>
> http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
>
> + 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.
Hi Damitri,
thanks for your time to review, I will fix these things according
to llvm coding style doc.
---
Regards,
Jiong
>
> Dmitri
>
--
Regards,
Jiong. Wang
Tilera Corporation.
More information about the cfe-dev
mailing list