This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][test] Cover loongarch in string.capacity test
AbandonedPublic

Authored by ldionne on Dec 4 2022, 11:30 PM.

Details

Reviewers
philnik
xen0n
xry111
MaskRay
SixWeining
Group Reviewers
Restricted Project

Diff Detail

Event Timeline

SixWeining created this revision.Dec 4 2022, 11:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 4 2022, 11:30 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
SixWeining requested review of this revision.Dec 4 2022, 11:30 PM
SixWeining added a subscriber: Ami-zhang.
MaskRay accepted this revision.Dec 5 2022, 2:27 PM
MaskRay added a reviewer: Restricted Project.
MaskRay added inline comments.
libcxx/test/libcxx/strings/basic.string/string.capacity/max_size.pass.cpp
87

I'd move loongarch before powerpc for a roughly alphabetical order.

This revision is now accepted and ready to land.Dec 5 2022, 2:28 PM
philnik added 1 blocking reviewer(s): Restricted Project.Dec 5 2022, 2:30 PM
This revision now requires review to proceed.Dec 5 2022, 2:30 PM
philnik added a subscriber: ldionne.

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

Hi @philnik. Yes, I tried to make libc++ usable on LoongArch. And now check-cxx 100% pass with this change. I'd like to hear what the community think.

FYI, check-cxxabi also pass with D138981.

xen0n added a comment.Dec 6 2022, 3:17 AM

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

I can confirm there's indeed desire to have libc++ supported on LoongArch. I'm Gentoo developer maintaining the LoongArch port, and we currently have automated LLVM/Clang stages (prebuilt bootstrapped sysroot tarballs) available for almost every supported architecture; it's nice to have feature parity on LoongArch too.

And from another perspective as an unaffiliated/independent watcher, the userbase of this architecture is real (although many don't speak English), and the arch is slowly gaining traction at least in China. So the Loongson Corporation probably has incentive to keep the LLVM/Clang/libc++ toolchain in good shape too, given a certain tendency to avoid GPL things (it's certainly not limited to certain Western tech giants).

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

Hi @philnik. Yes, I tried to make libc++ usable on LoongArch. And now check-cxx 100% pass with this change. I'd like to hear what the community think.

FYI, check-cxxabi also pass with D138981.

Having LoongArch work and having it supported are two different things. Having support means that you have to provide a libc++ pre-commit CI runner and contact person/team in case there is some problem with the runner itself or the platform.

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

I can confirm there's indeed desire to have libc++ supported on LoongArch. I'm Gentoo developer maintaining the LoongArch port, and we currently have automated LLVM/Clang stages (prebuilt bootstrapped sysroot tarballs) available for almost every supported architecture; it's nice to have feature parity on LoongArch too.

And from another perspective as an unaffiliated/independent watcher, the userbase of this architecture is real (although many don't speak English), and the arch is slowly gaining traction at least in China. So the Loongson Corporation probably has incentive to keep the LLVM/Clang/libc++ toolchain in good shape too, given a certain tendency to avoid GPL things (it's certainly not limited to certain Western tech giants).

You don't have to convince me that it should be supported. If you want to create your own processor and have it supported by libc++ you only have to provide enough compute resources to run the tests and a contact. If Loongson is interested they have to provide a runner; otherwise there is no way for us to know if anything we change actually works on the platform.

xen0n added a comment.Dec 6 2022, 4:17 AM

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

Hi @philnik. Yes, I tried to make libc++ usable on LoongArch. And now check-cxx 100% pass with this change. I'd like to hear what the community think.

FYI, check-cxxabi also pass with D138981.

Having LoongArch work and having it supported are two different things. Having support means that you have to provide a libc++ pre-commit CI runner and contact person/team in case there is some problem with the runner itself or the platform.

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

I can confirm there's indeed desire to have libc++ supported on LoongArch. I'm Gentoo developer maintaining the LoongArch port, and we currently have automated LLVM/Clang stages (prebuilt bootstrapped sysroot tarballs) available for almost every supported architecture; it's nice to have feature parity on LoongArch too.

And from another perspective as an unaffiliated/independent watcher, the userbase of this architecture is real (although many don't speak English), and the arch is slowly gaining traction at least in China. So the Loongson Corporation probably has incentive to keep the LLVM/Clang/libc++ toolchain in good shape too, given a certain tendency to avoid GPL things (it's certainly not limited to certain Western tech giants).

You don't have to convince me that it should be supported. If you want to create your own processor and have it supported by libc++ you only have to provide enough compute resources to run the tests and a contact. If Loongson is interested they have to provide a runner; otherwise there is no way for us to know if anything we change actually works on the platform.

Okay. I believe Loongson should be in a position, and willing, to provide such CI runners; they already applied for inclusion of one instance in D138672. Let's wait for @SixWeining's reply then.

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

Hi @philnik. Yes, I tried to make libc++ usable on LoongArch. And now check-cxx 100% pass with this change. I'd like to hear what the community think.

FYI, check-cxxabi also pass with D138981.

Having LoongArch work and having it supported are two different things. Having support means that you have to provide a libc++ pre-commit CI runner and contact person/team in case there is some problem with the runner itself or the platform.

Do you have plans to make libc++ support LoongArch? If not, this would introduce essentially dead code, which we are working very hard on removing from the code base. @ldionne WDYT about this?

I can confirm there's indeed desire to have libc++ supported on LoongArch. I'm Gentoo developer maintaining the LoongArch port, and we currently have automated LLVM/Clang stages (prebuilt bootstrapped sysroot tarballs) available for almost every supported architecture; it's nice to have feature parity on LoongArch too.

And from another perspective as an unaffiliated/independent watcher, the userbase of this architecture is real (although many don't speak English), and the arch is slowly gaining traction at least in China. So the Loongson Corporation probably has incentive to keep the LLVM/Clang/libc++ toolchain in good shape too, given a certain tendency to avoid GPL things (it's certainly not limited to certain Western tech giants).

You don't have to convince me that it should be supported. If you want to create your own processor and have it supported by libc++ you only have to provide enough compute resources to run the tests and a contact. If Loongson is interested they have to provide a runner; otherwise there is no way for us to know if anything we change actually works on the platform.

Okay. I believe Loongson should be in a position, and willing, to provide such CI runners; they already applied for inclusion of one instance in D138672. Let's wait for @SixWeining's reply then.

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Setting one up is documented at https://libcxx.llvm.org/AddingNewCIJobs.html. The requirement is implied at https://libcxx.llvm.org/#platform-and-compiler-support, but I'm not sure we actually explicitly state anywhere that we require a pre-commit runner for official support.

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Did @philnik's link help you or do you need assistance? If so the best place to ask in #libcxx on LLVM's Discord server.

SixWeining planned changes to this revision.Dec 18 2022, 11:58 PM

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Did @philnik's link help you or do you need assistance? If so the best place to ask in #libcxx on LLVM's Discord server.

Thank you and @philnik. I found we can't meet the compiler requirement (e.g. clang 14/15) currently. So let me mark Plan Change and reopen it after the requirement is updated to clang 15/16. Thanks.

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Did @philnik's link help you or do you need assistance? If so the best place to ask in #libcxx on LLVM's Discord server.

Thank you and @philnik. I found we can't meet the compiler requirement (e.g. clang 14/15) currently. So let me mark Plan Change and reopen it after the requirement is updated to clang 15/16. Thanks.

If clang 14 lacks some loongarch support it's also not a problem to only support loongarch for 15-and-up. AFAIK we also only test ARM for clang 15.

Yes, but D138672 is for post-commit. Libc++ needs a runner for pre-commit? Sorry, I haven't seen any docs about setting up pre-commit runner, specially for libc++. Could you please point it out? Thanks.

Did @philnik's link help you or do you need assistance? If so the best place to ask in #libcxx on LLVM's Discord server.

Thank you and @philnik. I found we can't meet the compiler requirement (e.g. clang 14/15) currently. So let me mark Plan Change and reopen it after the requirement is updated to clang 15/16. Thanks.

If clang 14 lacks some loongarch support it's also not a problem to only support loongarch for 15-and-up. AFAIK we also only test ARM for clang 15.

In fact loongarch gets minor support since clang 15 and I'm afraid it's not sufficient. At least we should wait clang 16.

It looks like you folks are on board with providing a pre-commit CI bot for this configuration? Please reach out to me on Discord if you want a BuildKite agent token so you can connect your machine(s) to our system.

ldionne commandeered this revision.Sep 7 2023, 10:39 AM
ldionne edited reviewers, added: SixWeining; removed: ldionne.

[Github PR transition cleanup]

Abandoning since this is blocked on upgrading to a supported compiler IIUC. Please reopen as a Github PR in the future if you want to do this.

ldionne abandoned this revision.Sep 7 2023, 10:39 AM