I’m sharing an initial set of patches that adds LLVM backend for RL78 architecture.
I’ll prepare the patches incrementally, in a similar order to Xtensa.
I’ve done this once before (⚙ D150693 [RL78 patch 1] Recognize RL78 in triple parsing code).
However, I took a break to update the LLVM version I was using as I was stuck with an old version so porting patches to latest sources seemed difficult.
Now my patches should in quicker succession.
if there are any concerns/queries please let me know.
Thank you very much!
Yes, of course.
Regarding my current setup, I’m using gitlab and I have a dedicated machine for the Runner and this is more than enough to build and run the full test suite(on which I can provide details as well if you whish!) on every single commit.
However, I’m probably averaging less than 1 commit per day (with spikes of maybe 2 or more commits on some occasions). And reading through: How To Add Your Build Configuration To LLVM Buildbot Infrastructure — LLVM 20.0.0git documentation
More specifically: “Thus, as a rule of thumb, we should plan for our builder to complete ~10-15 builds an hour.”
I can definitely say the current setup is NOT enough, it can build LLVM in ~7mins however the full test suite run pushes the time up to ~4.5 hours, so I will need to talk to management and get back to you. As it says in “Restrict what you build and test” I won’t need to run the test suite on every single commit like I do now which will further reduce this but still I will try to see what I can get.
Sorry, I also have a question: I knew for a long time I have to do this at some point, however I didn’t expect to look into this until I have landed most of my patches. Is your expectation different about this?
I have no intentions of letting it “rot”. I’m releasing the LLVM RL78 toolchain since 2021 and it has been a great success, the download figures have far exceeded those of GCC RL78. From 2021 to end of 2023 the toolchain was based on LLVM 10.0.0 and it took over 6 months to update to LLVM 17.0.1 (on which the latest releases from 2024 are based on). As you can imagine I would prefer to spend the time on implementing new things instead of updating the LLVM version I’m based on. So I’m highly motivated on upstreaming and keeping things working and up to date.
Thank you for the RFC! We have a documented policy for adding new targets to LLVM: LLVM Developer Policy — LLVM 20.0.0git documentation which poses one question we should know the answer to up front: are you intending for this target to be experimental or official?
Once you have that answer in mind, I’d recommend adding details to the RFC summary (not as a comment) along the lines of what’s suggested by this bullet point from the documentation:
Send a request for comment (RFC) to the LLVM Discourse forums describing your target and how it follows all the requirements and what work has been done and will need to be done to accommodate the official target requirements. Make sure to expose any and all controversial issues, changes needed in the base code, table gen, etc.
cc @rengolin and @arsenm who I think do most of the reviews for new targets being upstreamed.
All new backends start out as experimental anyway, so whether the backend will become non-experimental at some future point in time isn’t really a question we need to (or really can) answer now.
@Sebastian.Perta Regarding the buildbot, as this would start out as an experimental target, the buildbot would only run in the staging environment anyway, i.e. not send out emails to the blamelist on failure. The buildbot would exist primarily for you and other people working on the target, rather than the wider LLVM community (that would change if the target wants to become non-experimental at a later point in time). With that in mind, I wouldn’t be overly worried if it needs to batch many commits into one build right now.
Thank you very much Aaron for helping me out.
Unfortunately, I have hundreds of potentially controversial changes and I’m happy to expose all of them upfront in the next couple of days. In this way I will find out what early where the problematic areas are.
Since there are so many and you want them in the summary I’m thinking of splitting them into several RFCs and link all of them into the summary of this RFC.
I’m thinking one for LLVM, one for LLD, one for libcxx and so on. Even for the test suites since I modified test cases to make them suitable for RL78.
I will probably have to create more than one for clang.
Is this OK?
I think we want to ensure that LLVM is willing to take the new target, and once we have that nailed down, we can start to explore the impacts on other tools (which are generally pretty straight-forward; most of the heavy lifting will likely be in LLVM). If you know of some things that seem like they could be contentious, it’d be good to talk about them in the RFC, but I don’t think you have to go to all the effort to stage a bunch of patches quite yet. (That said, go with whatever advice you get from LLVM regulars like @nikic@arsenm@rengolin)
I think it’s great to get additional targets upstream, we should definitely encourage this sort of contribution.
I don’t think you need to make an RFC for every change to non-target-specific code or test-cases. Yes, any change could get comments and debate, but I’d say only if you are proposing major changes (e.g. modifying LLVM IR semantics, proposing to add support for non-8-bit-bytes, etc.) do you need to bring that up separately from normal code-review.
I’d also suggest that if you have an existing fork of LLVM available, that you point to that as context for reviewers. Even if it’s based on an outdated version of LLVM, or contains hacks that you plan to clean up before submitting upstream, it’s often helpful to be able to see the full context of what’s going to be coming.
I think there’s a nonzero barrier to accepting new targets for a reason. Upstream targets, even experimental ones, should have some evidence of a community of users and that it’s not somebody’s toy project. Otherwise we’re cluttering the project with low-value targets and having to delete them when they’ve been dormant for N months/years.
That said, two things:
The RFC author did not ask to upstream the target, only some of the common-code changes. The common-code changes can be considered without accepting the new target per se.
This particular target seems to have at least a long-term maintainer, if not a community of other people to help out. If the common-code changes could be better justified by an in-tree experimental target, it seems like that would be an appropriate step.
I don’t think you need to make an RFC for every change to non-target-specific code or test-cases. Yes, any change could get comments and debate, but I’d say only if you are proposing major changes (e.g. modifying LLVM IR semantics, proposing to add support for non-8-bit-bytes, etc.) do you need to bring that up separately from normal code-review.
Thank you! I’m working on updating the summary and highlight only the major changes and when the time comes I will discus them in detail.
I’d also suggest that if you have an existing fork of LLVM available, that you point to that as context for reviewers.
Thank you for the suggestion. I should be able to do this next week.
Even if it’s based on an outdated version of LLVM, or contains hacks that you plan to clean
That’s exactly how it is :). It’s based on LLVM 17.0.1 release and I still have many TODO’s in the code to re-write things or guard the change to affect only my target.
I think there’s a nonzero barrier to accepting new targets for a reason. Upstream targets, even experimental ones, should have some evidence of a community of users and that it’s not somebody’s toy project.
The port is for customers of the following MCU:
It is backed up by the company. The users for this compiler are usually smaller customers who don’t want to pay for the commercial compilers or are familiar with LLVM/GCC and of course only non-safety critical (like automotive and medical solutions) users who use only the commercial compilers:
Is this enough evidence? If not please let me know.
The RFC author did not ask to upstream the target, only some of the common-code changes. The common-code changes can be considered without accepting the new target per se.
Apologies! If I wasn’t clear enough. I do want to upstream the target. I will make sure when I update the summary I’ll make this clear.
Thank you!
If they’re beneficial on their own, upstream, sure. (see below)
About adding new targets, @pogo59 is spot on: There is a cost and the bureaucracy is there to make sure we all accept paying that cost.
Exactly!
In the past, upstreaming a target and the changes needed for that to happen have been done together (ex. Commodore64 and some table-gen changes).
This is not a requirement IFF the changes are beneficial to the (current) upstream ecosystem on their own. But if they’re just requirements from your code, I’d do them together, as evidence of need like the fork @jyknight suggests.
In the previous case, the changes were discussed in tandem with the back-end addition and, when the BE was approved, some initial changes (without need for the support) came first, then the new features, then the rest of the target.
I’m not saying we must do the same, just that we need the context for the needs at the same time as the code that needs it, to make informed decisions.
It’s possible there’s a better way of doing something that one of the upstream people will propose that make the changes unnecessary, which is the whole point of RFCs.
I’d also suggest that if you have an existing fork of LLVM available
I uploaded my current sources here:
I will add this link to the summary as well when I change it.
I also tried to create a PR so the changes can be viewed easily however for some reason the create pull request button disappears (maybe because the change is to big)
If you think I should create the PR as well I could investigate further just let me know ((maybe I will have better luck with github CLI)
This is what I used when I tried to create the PR:
In the past, upstreaming a target and the changes needed for that to happen >have been done together (ex. Commodore64 and some table-gen changes).
Regarding llvm I’ve managed to keep the changes to a minimum, for example no tablegen changes. Most of my changes which require RFCs are in the clang. However, many of them are related to language extensions for compatibility with the proprietary compiler and they are not needed to get the port up and running: by default I implemented this as a replacement for GCC RL78 so users can write stuff like: __attribute__((interrupt))
And I don’t think there any need for an RFC for this, my implementation is the same as for other targets. However an RFC is definitely needed for: #pragma interrupt
So many RFC can happen after the port has been accepted upstream.
That being said I still have quite changes which are absolutely needed for the port to work so I will start working on those.
This is not a requirement IFF the changes are beneficial to the (current) >upstream ecosystem on their own
I have a couple of changes like this as well. For example I absolutely needed srec support in objcopy (only ihex was available in llvm 10), and I had to implement it. However, it looks like current upstream sources have srec support so I don’t need to upstream this anymore. I have others as well and I think will do a couple to get me goings with RFCs at least as they are isolated and easier to explain.
In the previous case, the changes were discussed in tandem with the back-end addition and, when the BE was approved, some initial changes (without need for the support) came first, then the new features, then the rest of the target.
Thank you! Please note I even offered to raise many of them upfront (Aaron said that’s not necessary “but I don’t think you have to go to all the effort to stage a bunch of patches quite yet”.