Jiong Wang
2013-Mar-02 01:19 UTC
[LLVMdev] 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.
Jiong Wang
2013-Mar-07 16:33 UTC
[LLVMdev] [RFC] TileGX, a new backend for Tilera's many core processor
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>
Joerg Sonnenberger
2013-Mar-07 17:52 UTC
[LLVMdev] [cfe-dev] [RFC] TileGX, a new backend for Tilera's many core processor
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
Dmitri Gribenko
2013-Mar-07 20:48 UTC
[LLVMdev] [RFC] TileGX, a new backend for Tilera's many core processor
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>*/
Reasonably Related Threads
- [LLVMdev] [RFC] TileGX, a new backend for Tilera's many core processor
- [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor
- [LLVMdev] [RFC] TileGX, a new backend for Tilera's many core processor
- [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor
- [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor