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 On 18/08/16 12:22, Renato Golin wrote:> On 18 August 2016 at 08:34, Ed Jones via llvm-dev > <llvm-dev at lists.llvm.org> wrote: >> 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: >> https://github.com/embecosm > > 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 >
On 18 August 2016 at 13:31, Ed Jones <ed.jones at embecosm.com> wrote:> 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
On 18/08/16 13:57, Renato Golin wrote:> 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 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 On 18/08/16 13:31, Ed Jones wrote:> 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 > > > On 18/08/16 12:22, Renato Golin wrote: >> On 18 August 2016 at 08:34, Ed Jones via llvm-dev >> <llvm-dev at lists.llvm.org> wrote: >>> 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: >>> https://github.com/embecosm >> >> 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 >>
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 On 18/08/16 16:57, Ed Jones wrote:> 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 >