Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add loongarch64 support #2765

Merged
merged 1 commit into from
Apr 28, 2022
Merged

Add loongarch64 support #2765

merged 1 commit into from
Apr 28, 2022

Conversation

zhaixiaojuan
Copy link
Contributor

The LoongArch architecture (LoongArch) is an Instruction Set
Architecture (ISA) that has a Reduced Instruction Set Computer (RISC)
style.
The documents are on:
https://loongson.github.io/LoongArch-Documentation/README-EN.html
https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Once you have added a loongarch target in rustc, you'll want to add it to ci/build.sh under RUST_LINUX_NO_CORE_TARGETS so that the target is tested in libc CI.

@@ -115,6 +117,9 @@ cfg_if! {
} else if #[cfg(any(target_arch = "riscv64"))] {
mod riscv64;
pub use self::riscv64::*;
} else if #[cfg(any(target_arch = "loongarch64"))] {
mod loongarch64;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you forget to commit loongarch64/mod.rs?

@zhaixiaojuan zhaixiaojuan force-pushed the master branch 2 times, most recently from faf158e to b409b73 Compare April 25, 2022 01:23
Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The CI changes should be added after the target has been added to rustc.

ci/build.sh Outdated
@@ -140,6 +140,7 @@ aarch64-fuchsia \
armv5te-unknown-linux-gnueabi \
armv5te-unknown-linux-musleabi \
i686-pc-windows-gnu \
loongarch64-unknown-linux-gnu \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed: it will cause CI to fail since the target hasn't been added to rustc yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a. I have removed loongarch64-unknown-linux-gnu in ci/build.rs.
b. Added loongarch64 support in libc-test and completed the test locally. But for libc-test is it better to submit a PR separately?
See the test result: zhaixiaojuan@467c34c
c. Adjusted the loongarch64/mod.rs file according to the test of libc-test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add it in a separate PR once loongarch64-unknown-linux-gnu is available on nightly rust.

@Amanieu
Copy link
Member

Amanieu commented Apr 28, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 28, 2022

📌 Commit 467c34c has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Apr 28, 2022

⌛ Testing commit 467c34c with merge d747e7d...

@bors
Copy link
Contributor

bors commented Apr 28, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: Amanieu
Pushing d747e7d to master...

@xen0n
Copy link
Contributor

xen0n commented Jun 9, 2022

Holy moly, I didn't even know the existence of Loongson's porting efforts until I was kindly pointed to rust-lang/rust#96971, by @xry111, knowing that such effort cannot begin before the kernel/libc ABI freeze... next time surely CC me ;-)

@xry111
Copy link

xry111 commented Jun 9, 2022

Some questions:

  1. LoongArch doesn't have any big endian variant and I think it will never have one (it's now 2022 :). So should we remove the code about big endian or keep it for some reason I don't know?
  2. LoongArch won't have some syscalls, for example newfstatat. But the reference to newfstatat exists in our code. Should we remove it or keep it for some reason?

@xen0n
Copy link
Contributor

xen0n commented Jun 9, 2022

Some questions:

1. LoongArch doesn't have any big endian variant and I think it will never have one (it's now 2022 :).  So should we remove the code about big endian or keep it for some reason I don't know?

Definitely. It's unlikely LoongArch would ever become bi-endian, byteswapping is so cheap with dedicated revb instructions, that full-scale big-endian operation just isn't worth it. Plus bitfield semantics and aliasing between integer widths (think about how offsets are the same for a i64 field in a struct, no matter how you access it, whether as i8 or i16 or i32; in BE the offsets are different which is mildly annoying.)

2. LoongArch won't have some syscalls, for example [newfstatat](https://sourceware.org/pipermail/libc-alpha/2022-June/139257.html).  But the reference to newfstatat exists in our code.  Should we remove it or keep it for some reason?

Definitely too, we don't want to ever see fstat and newfstatat in the new world. (@zhaixiaojuan: This is precisely why people usually avoid starting any porting work before the ABI is officially frozen; if you ever followed the conversation at LKML you'd removed the fstat bits. We're in no hurry.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants