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

Some riscv32-linux porting work #20389

Merged
merged 23 commits into from
Jul 29, 2024
Merged

Some riscv32-linux porting work #20389

merged 23 commits into from
Jul 29, 2024

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Jun 22, 2024

  • Build and link musl
  • Build and run C hello world w/ musl
  • Build and run Zig hello world w/o libc
  • Build and run Zig hello world w/ musl

glibc work (if necessary) will be done in a later PR once #18803 is merged.

lib/std/start.zig Outdated Show resolved Hide resolved
lib/std/os/linux.zig Outdated Show resolved Hide resolved
@alexrp
Copy link
Member Author

alexrp commented Jun 22, 2024

  • Update std.posix to deal with the lack of fstat/fstatat

Open to ideas on this. My initial thinking is that I'll just have to update the std.posix functions to fall back to statx when fstat/fstatat aren't available, and translate the Statx structure to a Stat structure. This is what I did.

@alexrp
Copy link
Member Author

alexrp commented Jun 22, 2024

I suspect getting glibc working may require #18803.

/home/alexrp/Source/zig/build/stage3/bin/zig cc main.c -target riscv32-linux-gnuilp32.2.39.0
/home/alexrp/Source/zig/build/stage3/lib/zig/libc/glibc/sysdeps/riscv/start.S:48:2: error: unknown directive
 .cfi_label .Ldummy/home/alexrp/Source/zig/build/stage3/bin/zig build-exe main.zig -target riscv32-linux-gnuilp32 -lc
error(compilation): clang failed with stderr: /home/alexrp/Source/zig/build/stage3/lib/zig/libc/glibc/sysdeps/riscv/start-2.33.S:48:2: error: unknown directive
 .cfi_label .Ldummy

@Rexicon226
Copy link
Contributor

Rexicon226 commented Jun 23, 2024

Keep in mind that we no longer have riscv64 nor riscv32 in the test matrix. These changes will need to be either manually verified, or verified by adding riscv32 into the test matrix locally and running the suite.

See:

zig/test/tests.zig

Lines 415 to 436 in 6026bbd

// Disabled until LLVM fixes their O(N^2) codegen.
// https://github.com/ziglang/zig/issues/18872
//.{
// .target = .{
// .cpu_arch = .riscv64,
// .os_tag = .linux,
// .abi = .none,
// },
// .use_llvm = true,
//},
// Disabled until LLVM fixes their O(N^2) codegen.
// https://github.com/ziglang/zig/issues/18872
//.{
// .target = .{
// .cpu_arch = .riscv64,
// .os_tag = .linux,
// .abi = .musl,
// },
// .link_libc = true,
// .use_llvm = true,
//},

on how to add it back locally.

lib/std/os/linux.zig Outdated Show resolved Hide resolved
@alexrp
Copy link
Member Author

alexrp commented Jun 26, 2024

Can run Zig hello world with musl and without libc now!

I don't think there's much else to be done here other than making sure that CI is green and doing some local testing to make sure I haven't regressed riscv64. Progress on glibc stuff depends on #18803, but I don't think that necessarily has to block this PR.

So, I guess this is basically ready for review.

@alexrp alexrp marked this pull request as ready for review June 26, 2024 01:28
@alexrp alexrp requested a review from kprotty as a code owner June 26, 2024 01:28
lib/std/os/linux.zig Outdated Show resolved Hide resolved
lib/std/os/linux.zig Outdated Show resolved Hide resolved
lib/std/posix.zig Outdated Show resolved Hide resolved
@alexrp alexrp force-pushed the riscv32 branch 3 times, most recently from f2c7ff3 to 9d83b52 Compare July 9, 2024 14:06
Copy link
Contributor

@Rexicon226 Rexicon226 left a comment

Choose a reason for hiding this comment

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

Overall it seems fine to me - Just a couple of places where clarification would be nice.

lib/c.zig Show resolved Hide resolved
lib/std/Thread.zig Outdated Show resolved Hide resolved
lib/std/os/linux.zig Show resolved Hide resolved
lib/std/os/linux/syscalls.zig Show resolved Hide resolved
lib/std/os/linux/test.zig Outdated Show resolved Hide resolved
lib/std/posix.zig Outdated Show resolved Hide resolved
lib/std/posix.zig Outdated Show resolved Hide resolved
lib/std/os/linux/riscv32.zig Outdated Show resolved Hide resolved
@alexrp
Copy link
Member Author

alexrp commented Jul 20, 2024

Rebased with adjustments for #20679 and (I think) all review comments from @Rexicon226 addressed. Thanks for the review!

Now let's hope CI is still green. 🙂

alexrp added 21 commits July 29, 2024 09:50
Importantly, this ensures that the compiler understands that these ABIs need glibc.
Relevant release notes:

* https://sourceware.org/legacy-ml/libc-announce/2018/msg00000.html
* https://sourceware.org/pipermail/libc-announce/2021/000030.html

Note that the supported ISAs/ABIs are explicitly listed in each, and only 2.33
actually supports the 32-bit ones.
…ports.

If we're going to abuse the preprocessor, we may as well go all the way and have
it generate a convenient format for us. This achieves two things:

1. We no longer need hacks for the arch-specific syscalls.
2. We now generate the correct syscall names for 32-bit platforms.

The latter is because we now resolve __SC_3264, etc.
loongarch64 syscalls not updated because it seems like that kernel port hasn't
been working for a year or so:

    In file included from arch/loongarch/include/uapi/asm/unistd.h:5:
    include/uapi/asm-generic/unistd.h:2:10: fatal error: 'asm/bitsperlong.h' file not found

That file is just missing from the tree. :shrug:
No fstatat(), so there's no point doing the rest of it.
This is kind of a hack because the timespec in UAPI headers is actually still
32-bit while __kernel_timespec is 64-bit. But, importantly, all the syscalls
take __kernel_timespec from the get-go (because riscv32 support is so recent).

Defining our timespec this way will allow all the syscall wrappers in
std.os.linux to do the right thing for riscv32. For other 32-bit architectures,
we have to use the 64-bit time syscalls explicitly to solve the Y2038 problem.
This does not seem to be needed anymore, and it's unclear if it was ever truly
needed or if it was just there to deal with a QEMU/strace bug.

See: ziglang#8301 (comment)
…Linux.

statx() is strictly superior to stat() and friends. We can do this because the
standard library declares Linux 4.19 to be the minimum version supported in
std.Target. This is also necessary on riscv32 where there is only statx().

While here, I improved std.fs.File.metadata() to gather as much information as
possible when calling statx() since that is the expectation from this particular
API.
@andrewrk
Copy link
Member

Great work!

glibc work (if necessary) will be done in a later PR once #18803 is merged.

If you are willing to do the mentorship/review work on that PR, I am happy to merge it when you approve it.

@andrewrk andrewrk merged commit 38e0f04 into ziglang:master Jul 29, 2024
10 checks passed
@alexrp
Copy link
Member Author

alexrp commented Jul 29, 2024

If you are willing to do the mentorship/review work on that PR, I am happy to merge it when you approve it.

I still need to do a bit of reading to familiarize myself with how Zig handles glibc overall; will take a look at it then, hopefully soon.

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

Successfully merging this pull request may close these issues.

4 participants