llvm dev - Mar 2013 - [LLVMdev] [cfe-dev] [RFC] TileGX, a new backend for Tilera's many core processor

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.

Hi all,

Updated the patches for TILE-Gx backend:

1. added initial regression tests for tilegx codegen.
2. added initial regression tests for MC Layer.
3. fixed those commenting style issues.

please review, thanks.

I have tried to understand the new backend requirement for LLVM from the 
mailiing list archive,
it's sure TILE-Gx backend will be actively maintained & improved,
it's
not a code-drop'd backend :)

please feel free to give suggestions on how could this target get 
included in LLVM mainline.

=== Backend Intro ==
TILE-Gx is a VLIW architecture with 64-bit registers, 64-bit address space,
and 64-bit instructions. TILE-Gx has load-store architecture ISAs.

More information on the architectures is available at
http://www.tilera.com/scm/docs/index.html.

features supported
---
1. general function.
2. PIC/TLS/JumpTable.
3. Instructoin Bundling for VLIW.
4. Asm Parser (no support for bundle syntax .s, need modification on generic MC
code to support this)
5. MC Layer (support instruction bundle), MCJIT support.

regression result
---
Expected Passes : 13318
Expected Failures : 78
Unsupported Tests : 68
Unexpected Failures: 32
(20 failures are caused by lacking old JIT support)

test-suite result
---
Expected Passes : 949
Unexpected Failures: 17
(all 17 failures has the same output as tilegx gcc,
  most of them are about float precision issue)

---
Regards,
Jiong
Tilera Corporation

于 2013/3/2 9:19, Jiong Wang 写道:
> 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
>>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tilegx-backend.tar.bz2
Type: application/octet-stream
Size: 73509 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130308/6014a731/attachment.obj>

On Fri, Mar 08, 2013 at 12:33:58AM +0800, Jiong Wang
wrote:
> please review, thanks.
configure.ac: I guess it would be preferable to not enable it by default
for the moment.

include/llvm/Support/FEnv.h: What is supposed to handle? Build on/for
Tile? Does the platform provide fenv.h but in a broken way?

lib/Target/Mips/MCTargetDesc/MipsMCAsmInfo.cpp: Is this change
intentional?

Personally, I would like to see this in the tree sooner than later :)

Joerg

On Thu, Mar 7, 2013 at 6:33 PM, Jiong Wang <jiwang at tilera.com>
wrote:
> Hi all,
>
> Updated the patches for TILE-Gx backend:
>
> 1. added initial regression tests for tilegx codegen.
> 2. added initial regression tests for MC Layer.
> 3. fixed those commenting style issues.
>
> please review, thanks.
This is a huge patch, and reviewing it in tar.gz is hard.  To
facilitate review process, you can upload this to phabricator.

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>*/