[RFC] AAP Backend

Hi all,

We wish to submit our latest AAP implementation as an experimental
backend into LLVM. We need community feedback and reviewers for patches
which we will submit soon.

AAP was designed in early 2015 and aims to advance compiler development
for small deeply embedded Harvard architectures, which are widely used
commercially. AAP is freely available as an open source softcore for use
in FPGA designs.

AAP has wide exposure: at ORCONF16 at CERN, at FOSDEM and at BCS and
OSHUG meetings. It is also of commercial interest, because of the
potential benefits of providing upstream LLVM support for features found
in small embedded processors.

Currently there is a complete Clang and LLVM toolchain including gdb, ld
and binutils, as well as two simulator implementations. The toolchain is
passing all LLVM regression tests and a decent proportion of GCC
regression tests. We are also working on a GCC port to allow comparison
between compilers, which should be beneficial to both projects.

We believe the code base is sufficiently mature that it is appropriate
for inclusion. Currently, the full source for AAP can be found on Github:

Details about the ISA, and the hardware implementation can be found on
our website:
http://www.embecosm.com/resources/appnotes/#EAN13
http://www.embecosm.com/resources/appnotes/#EAN14

We are also planning to talk about AAP at the LLVM Cauldron in Hebden
Bridge. We look forward to discussing our work on AAP with those who are
attending.

Thank you,
Ed Jones

Hi Ed,

I had a look at the GitHub project and it wasn't clear what is needed
to move the AAP target in-tree.

First, it seems you're merging our tree into yours, which means the
merge commit [1,2] show the differences between old and new LLVM
stuff, not yours vs. upstream. This makes it hard to predict the
affected changes.

Also, you seem to have a standard-looking lib/Target/AAP [3], which is
a good step forward, but you also have added the simulator [4] in
there, which is not the right place. This should probably be a
different project altogether.

So, from our recent check-list for new targets, I'm assuming a few things:

Code owner: You?
Community: Embecosm + its users
License: AFAICS, check.
Docs / Impl: check

The thing that isn't clear right now is "compatible code and policies".

I don't see why we shouldn't take the AAP target, unless it poses a
big enough change to IR, the middle end, etc. So, I recommend two
steps:

1. A quick summary of the target independent changes you had to do to
make your back-end work. Changes to IR semantics, additional Clang
checks, base classes overwritten, etc.

2. Propose some patches on Phabricator, showing those differences.
These patches don't need to be the whole thing, for now, but basic
support ("hello world" style) should be working and tested.

We can continue from there.

cheers,
--renato

[1] https://github.com/embecosm/aap-llvm/commit/1d767ab10d0de413c341fe69ca228b97e7e1d255
[2] https://github.com/embecosm/aap-clang/commit/d1c2e5081a6222f2b4ab8d175fbd68a5e803c6d9
[3] https://github.com/embecosm/aap-llvm/tree/aap-master/lib/Target/AAP
[4] https://github.com/embecosm/aap-llvm/tree/aap-master/lib/Target/AAPSimulator

Hi Renato,

Currently I am building a set of patches which will add AAP piece-wise.
I'm following the approach that AVR (and now RISC-V), and the patches I
plan on adding are as follows:

* Target triple
* ELF definition
* Basic skeleton with the required build system changes (targetinfo +
target machine)
* Instruction + Register tablegen
* MC layer support
* AsmParser
* InstPrinter
* Disassembler
* Bulk of the implementation for lowering, isel and similar

At the moment there are only two changes to generic code, one is to
override the size of pointers when printing assembly (for AAP we use
larger pointer as the upper bits are used for flags), the other is to
track live outs of a function (We have some registers which are callee
saved, but also used for return values). I'm going to extract these
changes into separate patches and submit them along with the above.

We're not planning on submitting the simulator yet, we have been keeping
it outside of our main Target directory so that we don't write anything
that unintentionally relies on the simulator or lets parts of it leak
into any patches we submit. We may submit it separately at a later
point, as part of the AAP target directory.

I imagine either I or my colleague Simon Cook would be the code owner
(or both if that's possible). The community is Embecosm, and those who
may wish to use AAP as a basis for their work.

I will have a few basic patches submitted by the end of the day.

Thank you,
Ed Jones

Currently I am building a set of patches which will add AAP piece-wise.
I'm following the approach that AVR (and now RISC-V), and the patches I
plan on adding are as follows:

Sounds like a plan.

At the moment there are only two changes to generic code, one is to
override the size of pointers when printing assembly (for AAP we use
larger pointer as the upper bits are used for flags), the other is to
track live outs of a function (We have some registers which are callee
saved, but also used for return values). I'm going to extract these
changes into separate patches and submit them along with the above.

So, the discussion that has to happen before is: are those two
differences deal breakers for your target?

If the AAP back-end can't work without those two features, and they
prove hard to adapt, it may be hard to get the AAP in-tree.

But I don't think that will be the case. The CHERI back-end also has
fat pointers by default, and they were encouraged to merge their
changes, which is at least one similarity with your own back-end.

My personal take is that it would be good to allow non-integer
pointers in IR, but this may also prove complicated for the
middle-end. Time will tell.

Submitting those two patches, or at least their RFCs, would be a good
way to make sure there are no contentious issues with the rest of the
code.

Though, this should not stop you from putting up the actual patches
for review, and as soon as all the initial patches are approved on
their own merits, the target should be good to go.

We're not planning on submitting the simulator yet, we have been keeping
it outside of our main Target directory so that we don't write anything
that unintentionally relies on the simulator or lets parts of it leak
into any patches we submit. We may submit it separately at a later
point, as part of the AAP target directory.

I don't think that's a good idea. The simulator should be a separate
project altogether, on its own GitHub repository, using the LLVM
libraries as such.

I imagine either I or my colleague Simon Cook would be the code owner
(or both if that's possible). The community is Embecosm, and those who
may wish to use AAP as a basis for their work.

Should be fine.

cheers,
--renato

So, the discussion that has to happen before is: are those two
differences deal breakers for your target?

If the AAP back-end can't work without those two features, and they
prove hard to adapt, it may be hard to get the AAP in-tree.

But I don't think that will be the case. The CHERI back-end also has
fat pointers by default, and they were encouraged to merge their
changes, which is at least one similarity with your own back-end.

My personal take is that it would be good to allow non-integer
pointers in IR, but this may also prove complicated for the
middle-end. Time will tell.

Submitting those two patches, or at least their RFCs, would be a good
way to make sure there are no contentious issues with the rest of the
code.

I don't think they're absolutely necessary. Also when I say that we have
large pointers, it is only when emitting code. Pointers are 16-bits
internally, but we use a 32-bit ELF with the upper 16-bit used to store
flags such as the address space.

The other generic change is not absolutely necessary, and we could avoid
it by changing the calling convention.

I don't think that's a good idea. The simulator should be a separate
project altogether, on its own GitHub repository, using the LLVM
libraries as such.

Okay, this sounds sensible. We'll cross that bridge when we come to it.

Thank you,
Ed Jones

I don't think they're absolutely necessary. Also when I say that we have
large pointers, it is only when emitting code. Pointers are 16-bits
internally, but we use a 32-bit ELF with the upper 16-bit used to store
flags such as the address space.

Ahn, right, this makes it a lot simpler.

Well, may make your back-end more complicated, but makes upstreaming simpler. :slight_smile:

The other generic change is not absolutely necessary, and we could avoid
it by changing the calling convention.

It may not be necessary, let's see what the patches look like, so we
can discuss with actual code.

cheers,
--renato

Github makes it quite easy I think: https://github.com/embecosm/aap-llvm/compare/3951c48624d73169b5409c490999fadbdafd9fa4…1d767ab10d0de413c341fe69ca228b97e7e1d255

Nice! Thanks!

cheers,
--renato

I have now submitted the first 3 patches to add the AAP backend. These
add the target triple, ELF defines and the skeleton of a backend

https://reviews.llvm.org/D23664
https://reviews.llvm.org/D23665
https://reviews.llvm.org/D23667

Thank you,
Ed Jones

I have now submitted another 4 patches. These add register/instr
tablegen, MC layer support and AsmParser and InstPrinter support.

These additional patches mean that the backend can now execute some
basic MC encoding tests.

Thank you,
Ed Jones

I don't think we've ever really built up clear guidance on this, but I think there needs to be a clear determination that a given target has enough active users to make the maintenance burden worth putting into the mainline. In the past, the only exception I can think of is the Lanai backend, but in that case we have a strong commitment of multiple employees at a major corporation committed to that target's maintenance.

So, concretely, can you quantify the active customers for this target?

Alex

My interpretation of the current guidelines is that the "3 month coll
down period" is to make sure the community is active and addresses the
problems quickly and effectively enough. Ie. not something for making
targets experimental, but certainly for making them official, and
keeping the status later on.

cheers,
--renato

As it stands, the active customers for this target are the out-of-tree
backends which we are working on which can't be submitted for inclusion
into LLVM.

The general aim of the backend though is to include features from
architectures which are not well represented in LLVM, for example
non-power of two register sizes, non-octet chars, or very constrained
register sets, and to this end we hope for it to be useful to the
community at large if they are maintaining out-of-tree targets with
features they would like LLVM to support.

Thanks,
Ed

Adding to this further, the 2nd and 4th patches have been updated. Some
tests have been added for AAP ELF files, the formatting in the tablegen
has been made more consistent, and a few small changes from the main AAP
development have been pulled in.

The first patch has been approved, but I am waiting for more input on
some of the subsequent patches before asking for it to be committed.

Thanks,
Ed

Hi Ed,

I don't think there is.

Can you quantify Lanai's customers? Is there a community behind it?
Last I check it was only Google, and there were "in the process of"
upstreaming their emulator.

I don't see the difference. And if the maintainers can cope with the
load during the 3 months cool down period, what's the difference?

cheers,
--renato

I think Alex’s point is that there needs to be a sufficient user base or a sufficient commitment of maintenance to warrant the burden of the backend on the community.

I don't think there is.

Can you quantify Lanai's customers? Is there a community behind it?
Last I check it was only Google, and there were "in the process of"
upstreaming their emulator.

This was addressed in Alex’s email: " In the past, the only exception I can think of is the Lanai backend, but in that case we have a strong commitment of multiple employees at a major corporation committed to that target's maintenance.”.

I don't see the difference. And if the maintainers can cope with the
load during the 3 months cool down period, what's the difference?

I don’t think the 3 months cool down period replaces in any way this pre-evaluation.
If a single developer / single user of a virtual architecture is active enough for 3 months that nothing really breaks, it does not make it an “active community”.

This was addressed in Alex’s email: " In the past, the only exception I can think of is the Lanai backend, but in that case we have a strong commitment of multiple employees at a major corporation committed to that target's maintenance.”.

So, are we picking features based on company size, now? That doesn't
make much sense in an open source project...

The current policy states:

"There must be an active community behind the target. This community
will help maintain the target by providing buildbots, fixing bugs,
answering the LLVM community’s questions and making sure the new
target doesn’t break any of the other targets, or generic code. This
behavior is expected to continue throughout the lifetime of the
target’s code."

No mention about the size or the amount of money their companies have,
nor demands it to be a company at all.

I don’t think the 3 months cool down period replaces in any way this pre-evaluation.
If a single developer / single user of a virtual architecture is active enough for 3 months that nothing really breaks, it does not make it an “active community”.

Er... This is what the current policy states:

"The target must have addressed every other minimum requirement and
have been stable in tree for at least 3 months. This cool down period
is to make sure that the back-end and the target community can endure
continuous upstream development for the foreseeable future."

If everyone else's code don't break their stuff, or if every breakage
is met with prompt fix and improvement on the test suite, it doesn't
matter how many people, who or how many they are.

If nothing really breaks after 3 months it means that their back-end
is really pretty well isolated and hardened to cope with most
front-end and middle end changes that will be thrown at them at
considerable volumes. That sounds pretty good to me.

cheers,
--renato

This was addressed in Alex’s email: " In the past, the only exception I can think of is the Lanai backend, but in that case we have a strong commitment of multiple employees at a major corporation committed to that target's maintenance.”.

So, are we picking features based on company size, now? That doesn't
make much sense in an open source project…

“Major corporation” does not mean size to me, I read it as “having a major involvement in the project”.

The current policy states:

"There must be an active community behind the target. This community
will help maintain the target by providing buildbots, fixing bugs,
answering the LLVM community’s questions and making sure the new
target doesn’t break any of the other targets, or generic code. This
behavior is expected to continue throughout the lifetime of the
target’s code."

No mention about the size or the amount of money their companies have,
nor demands it to be a company at all.

Note that the text mentions "active community”, which is what I’m asking about:

“the question is about who will use/develop/maintain this backend upstream in LLVM?"
"is there already an open-source community around this backend somewhere?"

“Major corporation” does not mean size to me, I read it as “having a major involvement in the project”.

Still, you're rejecting new developers because they haven't
contributed much before.

But if their back-end is upstream, than they'll contribute code
upstream for their changes on their back-end.

However, if we don't allow their back-end to be upstream, they won't
contribute to the project.

Looks like a self-defeating argument, and one that still doesn't agree
with the open source philosophy.

Not to mention that it's an argument that is not required by the
policy in any way.

“the question is about who will use/develop/maintain this backend upstream in LLVM?"

They have answered that question. Ed and Simon will be the active
maintainers. I imagine they have other developers around to help.

I don't see *any* concern here, nor any violation of the policy. Like
the Lanai back-end, the community is the maintainers. LGTM.

"is there already an open-source community around this backend somewhere?"

This is not a requirement of the policy, nor was a requirement to any
other back-end, so not a valid argument.

If we were to reject changes for not having an open source community
elsewhere, we'd be chopping a very large parts of LLVM off.

cheers,
--renato