This is an archive of the discontinued LLVM Phabricator instance.

[libc++][CI] Add AIX pipeline config
ClosedPublic

Authored by daltenty on Oct 7 2021, 2:57 PM.

Details

Reviewers
ldionne
sfertile
Group Reviewers
Restricted Project
Commits
rG28b3cac7cf40: [libc++][CI] Add AIX pipeline config
Summary

This changes adds the pipeline config for both 32-bit and 64-bit AIX targets. As well, we add a lit feature LIBCXX-AIX-FIXME which is used to mark the failing tests which remain to be investigated on AIX, so that the CI produces a clean build.

Diff Detail

Event Timeline

daltenty created this revision.Oct 7 2021, 2:57 PM
daltenty requested review of this revision.Oct 7 2021, 2:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2021, 2:57 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I aborted the build since the AIX agents seem not to be available.

daltenty updated this revision to Diff 380122.Oct 15 2021, 5:28 PM
  • Add a feature marker for libcxx on AIX known failures which are still under investigation
  • XFAIL known failures
  • Mark tests which pass based on bitmode setting unsupported instead
daltenty updated this revision to Diff 380127.Oct 15 2021, 5:57 PM
  • Apply clang-format
daltenty updated this revision to Diff 380129.EditedOct 15 2021, 6:37 PM
  • Touch comment to retrigger CI
daltenty edited the summary of this revision. (Show Details)Oct 18 2021, 7:01 AM
ldionne requested changes to this revision.Oct 18 2021, 1:00 PM

This is awesome! I see that you currently have a single aix builder, and it takes about 45 minutes to run the tests. I can imagine that becoming a bottleneck -- do you think there is any way we could increase the capacity on aix builders?

Also, you could add AIX to the set of supported platforms in libcxx/docs/index.rst in this commit, since you are adding a CI job for it.

Finally, I assume there's a commitment to trying to fix the remaining failures on that platform?

libcxx/utils/ci/buildkite-pipeline.yml
693

Is it possible to handle that by using a different target triple instead? That would be consistent with what we try to do elsewhere.

694–695

Would it be possible to assign those builders to the libcxx-builders queue?

This revision now requires changes to proceed.Oct 18 2021, 1:00 PM
daltenty marked an inline comment as done.Oct 19 2021, 12:40 PM

I see that you currently have a single aix builder, and it takes about 45 minutes to run the tests. I can imagine that becoming a bottleneck -- do you think there is any way we could increase the capacity on aix builders?

Let me see what I can do to tune the build machine, I'll try putting the build in a ramdisk and a few other tricks (I'll see what I can do about getting more resources, but that might be tougher). We might be able to retire some older builders on the same machine as well.

Also, you could add AIX to the set of supported platforms in libcxx/docs/index.rst in this commit, since you are adding a CI job for it.

I'll add the build compiler were testing here as well if that's ok with you.

Finally, I assume there's a commitment to trying to fix the remaining failures on that platform?

We're actively looking in to these known failures and have queued up patches to address quite a few of them (including D112087 and D112086 posted already). We've been posting them in small batches though so as not to bog down reviewers. We intend to keep making continual progress on this queue in the near future.

libcxx/utils/ci/buildkite-pipeline.yml
693

That's rather difficult at the moment unfortunately. While setting the target triple will work just fine for the compile steps, the rest of the platform toolchain looks at this env (or explicit mode flags) to decide whether we are targeting 32 or 64 bit on AIX, and fails on any non-matching inputs, so you'll just get a bunch of failures from ld and ar. Between CMake and the clang driver, we have to teach it to pass the appropriate mode flags in a lot of place still for this to work cleanly.

daltenty updated this revision to Diff 380759.EditedOct 19 2021, 12:46 PM
  • Add the builders to the libcxx-builders queue
  • Add AIX to the list of supported platforms
  • Add Open XL
ldionne added inline comments.Oct 20 2021, 2:35 PM
libcxx/docs/index.rst
108

Now that is a much larger commitment. What is Open XL? Is it based on Clang? If so, what the relationship with Clang?

I'm asking because we are just starting to get out of the weeds with our compiler support and moving on to more recent compilers. The last thing I want is tie our hands again supporting some compiler that we don't have control over. I thought you folks were using Clang.

What is the release cycle for Open XL like?

Sorry for all those questions, but I want to figure out what this commitment would involve before we make it officially. I want to move out of the "hand wavy support" realm and into "we say we support it therefore we DO support it". But that makes the commitment much more serious, and so I'd like to understand the parameters of it.

daltenty added inline comments.Oct 22 2021, 8:51 AM
libcxx/docs/index.rst
108

I definitely understand the concern, hopefully I can clarify a bit.

What is Open XL? Is it based on Clang? If so, what the relationship with Clang?

IBM XL Compilers completed the adoption of Clang/LLVM and just released 17.1, which is rebranded to Open XL Compilers. This release is based on LLVM 13.

I thought you folks were using Clang.

We probably need to use Open XL 17.1 as a build compiler for now on AIX because a stable LLVM release which includes all required AIX support hasn't been released yet.

What is the release cycle for Open XL like?

IBM has been further investing in LLVM and made commitments to AIX support. Although we won't be able to expose the detailed release schedule, we tend to regularly deliver new releases of Open XL to pick up new stuff and enhancements from the community.

ldionne accepted this revision.Oct 28 2021, 8:46 AM

This looks fine to me -- welcome to the family.

Can you please rebase onto main so we get a clean CI run? I'd like to keep the CI red for everyone. It will also allow us to assess the latency for CI on AIX and make sure it's not going to cause problems.

libcxx/docs/index.rst
108

IBM has been further investing in LLVM and made commitments to AIX support. Although we won't be able to expose the detailed release schedule, we tend to regularly deliver new releases of Open XL to pick up new stuff and enhancements from the community.

Okay, got it. As long as there's an intent to regularly update the compiler, I'm fine with that. Basically, I want to make sure that we don't start having a bunch of OpenXL specific hacks in the code base to support that compiler just because our documentation says we do. As long as we don't have to bend backwards to support it, I think it's all good. Note that I have the same attitude towards Apple's own compiler -- I've removed support for all but the latest AppleClang in order to simplify things, and if we suddenly stopped updating our compiler, it would be reasonable for the project to stop supporting it. Fortunately that's not going to happen, but I just want to stress that these measures aim to protect the health of the project and let us keep a decent development velocity.

This revision is now accepted and ready to land.Oct 28 2021, 8:46 AM

This looks fine to me -- welcome to the family.

Can you please rebase onto main so we get a clean CI run? I'd like to keep the CI red for everyone. It will also allow us to assess the latency for CI on AIX and make sure it's not going to cause problems.

Thanks very much!

I'll rebase and hopefully the tuning on the builder should make a bit of difference (I've also hopefully secured some additional build resources, but it may take a bit before those come online)

daltenty updated this revision to Diff 383957.Nov 1 2021, 9:12 PM

Rebase to trigger CI

Sorry, our CI has been really flaky lately. Hopefully you should be able to rebase onto main once https://reviews.llvm.org/D113112 has landed (give it 1-2 hrs), and your build should go through.

This revision was automatically updated to reflect the committed changes.