RFC: TileGX, a new backend for Tilera's many core processor

Hi,

On behalf of Tilera Corporation, I'd like to contribute llvm ports to
Tilera's TILE-Gx
architecture and wish this could be submitted to main llvm tree.

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
End-to-End Networking Solutions | NVIDIA.

the attached patches contains the following main features for tilegx
backend:

1. general function.
2. PIC/TLS/JumpTable.
3. Instructoin Bundling for VLIW.
4. Basic support for Asm Parser.
5. MC Layer (aware of VLIW), MCJIT support.

I've run the regression test and standalone test-suite natively on
TILE-Gx silicon. The
test results are:

regression

llvm-tilegx-port.tar.bz2 (67.5 KB)

From: "Jiong Wang" <jiwang@tilera.com>
To: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>, cfe-dev@cs.uiuc.edu
Sent: Thursday, February 28, 2013 6:09:20 PM
Subject: [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor

Hi,

On behalf of Tilera Corporation, I'd like to contribute llvm ports to
Tilera's TILE-Gx
architecture and wish this could be submitted to main llvm tree.

Jiong, I am happy to see the Tile backend being offered for upstream inclusion. Among other things, in the long run, this may help inform and motivate many-core capabilities in LLVM.

First, can you elaborate on the future maintenance and development plans for the target code? Do you plan to add SIMD support? Atomic memory operations?

Will you be able to setup a buildbot on this architecture? If so, can it be connected to the public system?

On the patch itself:

1. There are still a few places where there is commented-out code or #if 0 blocks; these should be removed (or replaced with real comments as appropriate).

2. There are no regression tests -- using the test-suite is obviously useful, but targeted regression tests are essential.

You say that there are unexpected failures in the regression tests and in the test suite. Do you understand these?

-Hal

Hi Hal, thanks for feedback. yes, currently we lack of old JIT support (I was thinking MCJIT will be the future, but from the mailinglist archive, it seems old JIT is still necessary), SIMD intrinsic support etc. all these will be supported next. As some of the llvm modules are in active development, for example MC Layer, we want to return code to community repository first, so that it will be easy to keep pace with llvm main tree. yes, I guess it’s OK to setup a tilegx buildbot with connection to public system. I will confirm this with the company. thanks, I will fix these things. yes, regression tests will be added later. yes, I have done investigation on these failures. regression (44 failures) === 20: caused JITTests failures, we lack old jit support 2: MCJIT/test-common-symbols-remote.ll MCJIT/test-fp-no-external-funcs-remote.ll the other 20 are mostly under Clang:: Tooling (these testcases seems to be added in this week) & some debuginfo testcases, I will clean up them later. test-suite (17 failures) ====== 1 of them are compile stage failures === MultiSource/Benchmarks/tramp3d-v4/tramp3d-v4 the reason is tilegx do not support some c99 float rounding & exceptions we lack the support of some flag like FE_DIVBYZERO etc, for fegetexceptflag. 16 of them are runtime failures. === actually, I compare all these failures with our gcc output, it’s the same. nearly all of them are caused by float point precision issue, for example, MultiSource/Applications/sqlite3/

===> besides these, for the next step, we will support more peephole optimizations, optimize float operation using special hardware instruction, implement a VLIW aware MI schduler etc, to improve instruction level parallelism. And I guess it will be interesting for how to support higher level parallelism using TileGX’s 72 core :slight_smile:

From: "Jiong Wang" <jiwang@tilera.com>
To: "Hal Finkel" <hfinkel@anl.gov>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>, cfe-dev@cs.uiuc.edu
Sent: Friday, March 1, 2013 1:34:15 AM
Subject: Re: [LLVMdev] RFC: TileGX, a new backend for Tilera's many core processor

Hi Hal,

thanks for feedback.

Jiong, I am happy to see the Tile backend being offered for upstream
inclusion. Among other things, in the long run, this may help inform
and motivate many-core capabilities in LLVM.

First, can you elaborate on the future maintenance and development
plans for the target code? Do you plan to add SIMD support? Atomic
memory operations? yes, currently we lack of old JIT support (I was
thinking MCJIT will be the future, but from the mailinglist archive,
it seems old JIT is still necessary), SIMD intrinsic support etc.
all these will be supported next.

As some of the llvm modules are in active development, for example MC
Layer, we want to return code to community repository first, so that
it will be easy to keep pace with llvm main tree.

I think this makes sense; but my impression is that the community will want a clear idea that this will be maintained and improved for the foreseeable future.

Will you be able to setup a buildbot on this architecture? If so, can
it be connected to the public system? yes, I guess it's OK to setup
a tilegx buildbot with connection to public system. I will confirm
this with the company.

On the patch itself:

1. There are still a few places where there is commented-out code or
#if 0 blocks; these should be removed (or replaced with real
comments as appropriate). thanks, I will fix these things.

2. There are no regression tests -- using the test-suite is obviously
useful, but targeted regression tests are essential. yes, regression
tests will be added later.

I don't speak for everybody, but I'm pretty sure that you won't get an okay to commit this upstream without regression tests. We need a good degree of coverage in test/CodeGen/Tile (you can probably look at one of the other more-recent targets, like AArch64, to get an idea of what is required).

-Hal

As some of the llvm modules are in active development, for example MC
Layer, we want to return code to community repository first, so that
it will be easy to keep pace with llvm main tree.
I think this makes sense; but my impression is that the community will want a clear idea that this will be maintained and improved for the foreseeable future.

      Hi Hal,

        sure, tilegx will be actively maintained & improved.

2. There are no regression tests -- using the test-suite is obviously
useful, but targeted regression tests are essential. yes, regression
tests will be added later.

I don't speak for everybody, but I'm pretty sure that you won't get an okay to commit this upstream without regression tests. We need a good degree of coverage in test/CodeGen/Tile (you can probably look at one of the other more-recent targets, like AArch64, to get an idea of what is required).

        thanks for pointting this out, I will add regression tests, then re-send the patch.
        before this, please feel free to give comments and suggestions on the existed patches.

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.

Dmitri

Hi Damitri,

        thanks for your time to review, I will fix these things according
to llvm coding style doc.

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 :slight_smile:

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
End-to-End Networking Solutions | NVIDIA.

features supported

tilegx-backend.tar.bz2 (71.8 KB)

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 :slight_smile:

Joerg

This is a huge patch, and reviewing it in tar.gz is hard. To
facilitate review process, you can upload this to phabricator.

Dmitri

于 2013/3/8 1:52, Joerg Sonnenberger 写道:

please review, thanks.

configure.ac: I guess it would be preferable to not enable it by default
for the moment.

I agree.

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?

yes, the platform provide fenv.h but in a broken way.

TileGX has incomplete support for fenv, we are lack of some macro define.

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

No. Sorry, my bad, it's not intentional.

OK, I am requesting for a phabricator account.

   one other things is, should I split the patch into several parts? like

   [PATCH 1/10]
   [PATCH 2/10]
    ...

   because clang patch & test-suite patch is relatively small, but llvm patch is nearly 10K line, still hard to review.

   if it is, will one patch for one file OK?

   What's the patch policy in llvm community ?

   thanks

Hi Jiong,

The LLVM community prefers small, self-contained patches. Please split up your patches into small chunks that can be easily reviewed. Keep in mind that the compiler needs to work after each pass goes in. :slight_smile:

-bw

Just like with AArch64, this is not a reasonable requirement for a new
backend. The tarball contains essentially three different parts:
(1) Generic changes to recogniz TileGX as triple etc.
(2) The target subdirectory.
(3) The clang logic for va_arg etc.

While (1) can be and should be split off, it doesn't change match in
terms of patch size, since the majority will stay in item (2).

Joerg

There are exceptions to every rule. :slight_smile: The upshot is that it needs to be fairly easy to review.

-bw