[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