Details
- Reviewers
ldionne Mordante curdeius - Group Reviewers
Restricted Project - Commits
- rG3aa6a4cb39c4: [libcxx][Arm] Move buildbot flags into cmake files
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM, provided the build passes. But please wait for a libc++ approval.
A question how resource constrained are the Arm buildbots? The Apple builds will only build after the build before finish, this is done due to the fact the Apple build nodes are quite slow. If Arm isn't resource constrained it might be an idea to move this build directly after the "Unified standalone build. @ldionne I'd also like to hear your opinion.
A question how resource constrained are the Arm buildbots?
We've got 6 agents for Arm, enough to build a single revision in parallel with ~40 minute average build time.
The Apple builds will only build after the build before finish, this is done due to the fact the Apple build nodes are quite slow
And (I assume) because there's some finite number of them. If you wanted a faster pipeline and had infinite (but slow) agents you would put the Apple agents in the first part.
If Arm isn't resource constrained it might be an idea to move this build directly after the "Unified standalone build
Whether 1 agent per config is "constrained" is gonna depend on how many builds get run and what your expectations are. Unless your gut reaction is "yes definitely" then I'd be happy to move them into the first set and find out, best way to find out.
(for the moment my logic is as long as they're as fast or faster than the Apple builders then we're not slowing down the pipeline by being in the second set)
| libcxx/cmake/caches/Armv7Arm.cmake | ||
|---|---|---|
| 2 | Just FYI, you might have seen triples like thumv7-linux-gnueabihf but these only apply to cc1. You can give those to clang but it'll convert them to armv7-linux-gnueabihf. That's why we need the extra -marm/-mthumb flags. | |
| libcxx/cmake/caches/Armv7Arm.cmake | ||
|---|---|---|
| 2 | (that should be thumbv7-linux-gnueabihf) | |
Sorry for changing my mind.
What about:
-DLIBCXX_ENABLE_EXCEPTIONS=OFF \ -DLIBCXXABI_ENABLE_EXCEPTIONS=OFF
They should be in cmake caches too, no?
Cf. generic-noexceptions that uses Generic-noexceptions.cmake.
I think it makes sense for those ARM builders to be executed only when the other jobs succeed since they don't have basically unlimited bandwidth. I think it almost only makes sense to put the jobs running on GCE in the first section.
Regarding:
I am ambivalent about that one. On the one hand, I agree this should be in a cache, however that could lead to a large number of caches if we always put each configuration in a cache of its own. I'd say let's go for a separate cache right now just so there's a 1-1 correspondence between CMake caches and supported configurations, but let's revisit this in the future if it doesn't make sense anymore.
There's one Apple agent in the first part. If that fails the other Apples aren't build. Apple is a bottleneck when a lot of builds are scheduled.
(for the moment my logic is as long as they're as fast or faster than the Apple builders then we're not slowing down the pipeline by being in the second set)
Yeah, I just wondered whether this was a conscious decision or not. Louis also thinks the current way is good so let's keep it that way.
- Move all flags into cmake caches.
One useful thing I found is that you can give cmake multiple cache
files. So you could use the Arm cache and the noexception cache.
Might come in handy later, 1:1 makes sense for the moment.
You're probably wondering where the Arm-noexception configs
are and that's intentional. Building arm only or thumb only isn't
supposed to be 100% coverage, just enough to pick up the obvious issues.
If that proves to be lacking then I can find some more resource
on our side to test those configs.
LGTM.
I think we'll do this "clean up" and use multiple cache files at some moment indeed.
Just FYI, you might have seen triples like thumv7-linux-gnueabihf but these only apply to cc1. You can give those to clang but it'll convert them to armv7-linux-gnueabihf. That's why we need the extra -marm/-mthumb flags.