initial arm64 support #1429

shawnl · 2018-08-27T12:58:20Z

_start
syscalls
clone
vdso

andrewrk · 2018-08-27T18:59:41Z

I'm happy to add arm support, but the biggest showstopper is #835 - if we can't get automated tests running on every pushed commit, we can't add support. The biggest step anyone can do toward getting zig to support another target is solving the CI problem.

shawnl · 2018-08-27T22:38:13Z

ok, the hello world builds in --release-fast mode, just not debug mode, as there are problems with the atomics (which compile out) and maybe other bugs. That is why I closed it, I wanted more to work

andrewrk · 2018-08-27T22:54:30Z

This PR depends on having CI for ARM.

shawnl · 2018-08-29T03:07:37Z

CI for arm https://buildbot.git.icu/buildbot/

builds on pushes to master, and if you set up github webhooks to

https://buildbot.git.icu/buildbot/change_hook/github (push)

then it will build on pull requests.

I don't know how to get it integrated in the github GUI.
I can add FreeBSD and OpenBSD when someone does the porting work.

shawnl · 2018-08-30T22:44:06Z

llvm is generating garbage assembly https://bugs.llvm.org/show_bug.cgi?id=38734 which this bug is blocked on.

also, the CI is timing out on windows and max osx.

andrewrk · 2018-08-30T23:38:21Z

This might be zig doing the C ABI incorrectly. LLVM sadly does not provide the C ABI. I would check the equivalent LLVM IR generated by clang and compare.

andrewrk · 2018-08-30T23:39:16Z

Oh, wait, that does seem like an llvm bug because your code is not doing the c ABI. Nevermind previous comment.

these don't exist on new platforms (such as arm64) also switch from the deprecated dirent to dirent64

arm64 complains about the old value (I added a test)

From IEEE-754 standard: Conversion of a quiet NaN in a supported format to an external character sequence shall produce a language-defined one of “nan” or a sequence that is equivalent except for case (e.g., “NaN”), with an optional preceding sign. (This standard does not interpret the sign of a NaN.)

Auxillery vectors are not guaranteed to be in any order, this just happens to work on x86_64.

shawnl · 2018-09-08T14:56:08Z

std/os/index.zig

    }
+    return 0;


should this return null, and make the return ?usize

The libc version doesn't distinguish, so the zig-native version matches the API.

andrewrk

This looks really good. I have a few things to address, and then it's ready for merge.

andrewrk · 2018-10-02T14:14:20Z

std/fmt/index.zig

@@ -1034,11 +1034,6 @@ test "fmt.format" {
        const result = try bufPrint(buf1[0..], "f64: {}\n", math.nan_f64);
        assert(mem.eql(u8, result, "f64: nan\n"));
    }
-    {


can you disable this test only for ARM?

if (builtin.arch ...

andrewrk · 2018-10-02T14:16:26Z

std/os/linux/index.zig

@@ -624,6 +625,54 @@ pub const S_IWOTH = 0o002;
 pub const S_IXOTH = 0o001;
 pub const S_IRWXO = 0o007;

+pub const O_CREAT = 0o100;


Although these constants are the same on x86_64 and aarch64, they are per-architecture values:

andy@xps ~/d/musl> grep -RI O_CREAT arch/mips64/bits/fcntl.h:#define O_CREAT 0400 arch/generic/bits/fcntl.h:#define O_CREAT 0100 arch/x32/bits/fcntl.h:#define O_CREAT 0100 arch/powerpc64/bits/fcntl.h:#define O_CREAT 0100 arch/arm/bits/fcntl.h:#define O_CREAT 0100 arch/s390x/bits/fcntl.h:#define O_CREAT 0100 arch/mips/bits/fcntl.h:#define O_CREAT 0400 arch/x86_64/bits/fcntl.h:#define O_CREAT 0100 arch/aarch64/bits/fcntl.h:#define O_CREAT 0100 arch/m68k/bits/fcntl.h:#define O_CREAT 0100 arch/mipsn32/bits/fcntl.h:#define O_CREAT 0400 arch/powerpc/bits/fcntl.h:#define O_CREAT 0100

I think they should stay in the per-architecture files to prevent bugs when more architecture support is added in the future.

andrewrk · 2018-10-02T14:22:41Z

std/os/linux/index.zig

@@ -1189,17 +1272,22 @@ pub fn accept4(fd: i32, noalias addr: *sockaddr, noalias len: *socklen_t, flags:
 }

 pub fn fstat(fd: i32, stat_buf: *Stat) usize {
-    return syscall2(SYS_fstat, @intCast(usize, fd), @ptrToInt(stat_buf));
+    return fstatat(fd, c"", stat_buf, AT_EMPTY_PATH);


I don't think this is really important, but, is there a benefit to not calling fstat? The previous version of this function does slightly less work, since it has to set up 2 fewer parameters, and it supports older kernels. (Same question for e.g. epoll_pwait.)

Drive-by comment: arm64 doesn't have an epoll_wait() syscall, only epoll_pwait(). libc emulates epoll_wait() as epoll_pwait() with a null sigmask, just like this PR does.

(Arm64 does have fstat() though, so no reason I can see to use fstatat().)

arm64 also has execve and execveat

andrewrk · 2018-10-02T14:37:08Z

std/os/index.zig

@@ -633,16 +633,21 @@ fn posixExecveErrnoToErr(err: usize) PosixExecveError {
    };
 }

-pub var linux_aux_raw = []usize{0} ** 38;
+pub var linux_elf_aux_maybe: ?[*]std.elf.Auxv = undefined;


it looks like you want this to be initialized to null, not undefined.

but it is always set at program initialization, initializing it to null requires more code in every executable. I make it a maybe because one might want to unmap the entire elf auxiliary vector to save space.

It is not set at program initialization when zig code is used as a library, with main in a C object, for example. It's also not set at program initialization when the user puts export fn main(argc: c_int, argv: [*][*]u8) c_int {...} and links with libc. Why would initializing it to null require more code? It's static data, so if it's null it would go in the .bss section.

andrewrk · 2018-10-02T14:42:54Z

I believe the current buildbot failures are due to building against llvm 6 instead of llvm 7.

andrewrk · 2018-10-02T17:21:57Z

src/os.cpp

@@ -839,9 +839,9 @@ static int os_exec_process_posix(const char *exe, ZigList<const char *> &args,
    if ((err = pipe(stderr_pipe)))
        zig_panic("pipe failed");

-    pid_t pid = fork();
+    pid_t pid = vfork();


This should be posix_spawn rather than vfork. See #1620

andrewrk · 2018-10-05T20:04:56Z

Looks good to me. Are you ready for me to merge?

andrewrk · 2018-10-05T20:05:32Z

By the way, it looks like the buildbot environment has the wrong llvm/clang version.

shawnl · 2018-10-06T03:05:51Z

I believe the current buildbot failures are due to building against llvm 6 instead of llvm 7.

That is the fault of this line:

cmake/Findllvm.cmake: NAMES llvm-config llvm-config-7 llvm-config-7.0

llvm-config on debian still points to llvm-config-6

should i prepare a patch?

shawnl · 2018-10-06T03:08:08Z

Looks good to me. Are you ready for me to merge?

yes

andrewrk · 2018-10-06T04:10:55Z

should i prepare a patch?

I just went ahead and fixed it in master. And I'll merge this PR too. Excellent work.

andrewrk · 2018-10-08T14:42:27Z

Looks like the build bot environment is not fixed by the Findllvm.cmake change

shawnl closed this Aug 27, 2018

shawnl reopened this Aug 27, 2018

shawnl closed this Aug 27, 2018

shawnl reopened this Aug 27, 2018

andrewrk added this to the 1.0.0 milestone Aug 27, 2018

andrewrk added the blocked label Aug 27, 2018

shawnl force-pushed the arm64 branch 3 times, most recently from c1a1012 to 8b6bd05 Compare August 28, 2018 11:40

shawnl force-pushed the arm64 branch 13 times, most recently from 2599ed7 to 789ee9e Compare August 30, 2018 18:36

shawnl force-pushed the arm64 branch from 789ee9e to e94c0ee Compare August 30, 2018 23:54

andrewrk added this to the 0.4.0 milestone Sep 3, 2018

shawnl force-pushed the arm64 branch 2 times, most recently from 30e22c9 to e7ca743 Compare September 8, 2018 03:37

shawnl added 7 commits September 8, 2018 03:52

this is not arch-specific

d956d30

os: use less syscalls

4a8c992

these don't exist on new platforms (such as arm64) also switch from the deprecated dirent to dirent64

initial arm64 support

342cff2

simplify f64_min to equivilent value

f8808ed

arm64 complains about the old value (I added a test)

clone() on arm64

cba0d76

use vfork in stage1 compiler to avoid OOM

fdeb876

shawnl force-pushed the arm64 branch from e7ca743 to 7ddaa12 Compare September 8, 2018 03:52

fix elf auxv handling

17cb69c

Auxillery vectors are not guaranteed to be in any order, this just happens to work on x86_64.

shawnl force-pushed the arm64 branch from 7ddaa12 to 17cb69c Compare September 8, 2018 14:47

shawnl commented Sep 8, 2018

View reviewed changes

andrewrk requested changes Oct 2, 2018

View reviewed changes

andrewrk reviewed Oct 2, 2018

View reviewed changes

shawnl force-pushed the arm64 branch 2 times, most recently from 1d8040b to f19d421 Compare October 5, 2018 18:40

andrewrk approved these changes Oct 5, 2018

View reviewed changes

arm64: respond to code review

2d27341

shawnl force-pushed the arm64 branch from f19d421 to 2d27341 Compare October 6, 2018 03:32

andrewrk merged commit d40c4e7 into ziglang:master Oct 6, 2018

valpackett mentioned this pull request Oct 18, 2018

[WIP] FreeBSD support #1661

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial arm64 support #1429

initial arm64 support #1429

shawnl commented Aug 27, 2018 •

edited

andrewrk commented Aug 27, 2018

shawnl commented Aug 27, 2018

andrewrk commented Aug 27, 2018

shawnl commented Aug 29, 2018

shawnl commented Aug 30, 2018 •

edited

andrewrk commented Aug 30, 2018

andrewrk commented Aug 30, 2018

shawnl Sep 8, 2018

andrewrk Sep 8, 2018

andrewrk left a comment

andrewrk Oct 2, 2018

andrewrk Oct 2, 2018

andrewrk Oct 2, 2018 •

edited

bnoordhuis Oct 2, 2018

shawnl Oct 5, 2018

andrewrk Oct 2, 2018

shawnl Oct 5, 2018

andrewrk Oct 5, 2018 •

edited

andrewrk commented Oct 2, 2018

andrewrk Oct 2, 2018

andrewrk commented Oct 5, 2018

andrewrk commented Oct 5, 2018

shawnl commented Oct 6, 2018

shawnl commented Oct 6, 2018

andrewrk commented Oct 6, 2018

andrewrk commented Oct 8, 2018

initial arm64 support #1429

initial arm64 support #1429

Conversation

shawnl commented Aug 27, 2018 • edited

andrewrk commented Aug 27, 2018

shawnl commented Aug 27, 2018

andrewrk commented Aug 27, 2018

shawnl commented Aug 29, 2018

shawnl commented Aug 30, 2018 • edited

andrewrk commented Aug 30, 2018

andrewrk commented Aug 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk Oct 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk Oct 5, 2018 • edited

Choose a reason for hiding this comment

andrewrk commented Oct 2, 2018

Choose a reason for hiding this comment

andrewrk commented Oct 5, 2018

andrewrk commented Oct 5, 2018

shawnl commented Oct 6, 2018

shawnl commented Oct 6, 2018

andrewrk commented Oct 6, 2018

andrewrk commented Oct 8, 2018

shawnl commented Aug 27, 2018 •

edited

shawnl commented Aug 30, 2018 •

edited

andrewrk Oct 2, 2018 •

edited

andrewrk Oct 5, 2018 •

edited