This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][Arm] Move buildbot flags into cmake files
ClosedPublic

Authored by DavidSpickett on Mar 17 2021, 4:22 AM.

Details

Diff Detail

Event Timeline

DavidSpickett created this revision.Mar 17 2021, 4:22 AM
DavidSpickett requested review of this revision.Mar 17 2021, 4:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2021, 4:22 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante accepted this revision as: Mordante.Mar 17 2021, 10:08 AM
Mordante added a subscriber: Mordante.

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)

DavidSpickett added inline comments.Mar 18 2021, 3:17 AM
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.

DavidSpickett added inline comments.Mar 18 2021, 3:19 AM
libcxx/cmake/caches/Armv7Arm.cmake
2

(that should be thumbv7-linux-gnueabihf)

curdeius accepted this revision.Mar 18 2021, 9:24 AM
curdeius added a subscriber: curdeius.

LGTM.

This revision is now accepted and ready to land.Mar 18 2021, 9:24 AM
curdeius requested changes to this revision.EditedMar 18 2021, 9:26 AM

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.

This revision now requires changes to proceed.Mar 18 2021, 9:26 AM

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:

Sorry for changing my mind.
What about:

-DLIBCXX_ENABLE_EXCEPTIONS=OFF \
-DLIBCXXABI_ENABLE_EXCEPTIONS=OFF

They should be in cmake caches too, no?

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.

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.

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.

curdeius accepted this revision.Mar 19 2021, 8:24 AM

LGTM.
I think we'll do this "clean up" and use multiple cache files at some moment indeed.

This revision is now accepted and ready to land.Mar 19 2021, 8:24 AM
This revision was automatically updated to reflect the committed changes.