-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
stage1 compiler for dragonfly os + ravenports #2381
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments. Nice work!
std/os/dragonfly.zig
Outdated
@@ -0,0 +1,845 @@ | |||
const builtin = @import("builtin"); | |||
|
|||
pub use @import("dragonfly/errno.zig"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question as for std/c/dragonfly.zig
: can this be shared with freebsd.zig? This is the only difference between those files, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confess to having checked the other files for differences. But I left this one (other that errno). If there are differences with freebsd, I'm maybe not the person to fix them -- never having used 99% of what's in that file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you copy this file from FreeBSD? I'm worried that there are hard-to-catch bugs in here if there is anything different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally yes it was copied. Now all defines are just wrappers around c/dragonfly.zig. Except for O_LARGEFILE. I initially special cased the dragonfly's absence of O_LARGEFILE in *.zig, but felt it was maybe better to just set O_LARGEFILE to the no-op of 0 for the moment
src/link.cpp
Outdated
"-L/raven/share/raven/toolchain/gcc8/lib"); | ||
// prefer .a over .so | ||
// fixes: lld: warning: found local symbol '__thread_locale' in global part of symbol table in file /raven/share/raven/sysroot/DragonFly/usr/lib/libc.so | ||
lj->args.append("/raven/share/raven/sysroot/DragonFly/usr/lib/libc.a"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a blocker, just a question from an interested onlooker: how would someone who uses dports rather than ravenports use zig on dragonflybsd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ravenports should work the same on linux/macos/solaris/freebsd/dragonfly and maybe netbsd. It looks like they will accept my port, if so, the command to install it will be
> /raven/sbin/pkg install zig-single-standard
And from there on it should be the same as dports -- though using /raven/lib etc rather than /usr/lib etc
These commits try to show the minimum change that gets the multithreaded tests working on dragonfly, but it does this by changing zigTriple() from With this commit, the tests on dragonfly all output something like
Does the Note: the tests with --single-threaded fail with |
Just a small note to add: I sometimes see |
Previously if you had, for example: extern "c" threadlocal var errno: c_int; This would turn errno into a normal variable for --single-threaded builds. However for variables with external linkage, there is an ABI to uphold. This is needed to make errno work for DragonFly BSD. See #2381.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tse-gratis,
Nice work! I'm excited to merge this.
build-exe works, but running the tests with build --build-file ../build.zig test-behavior for zig-0.4.0 gives multiple "skipping execution because it is non-native"
This is actually working fine - DragonFly BSD is not in the test matrix so it is not getting tested by test-behavior
:
Lines 31 to 47 in f6937db
const test_targets = []TestTarget{ | |
TestTarget{ | |
.os = builtin.Os.linux, | |
.arch = builtin.Arch.x86_64, | |
.abi = builtin.Abi.gnu, | |
}, | |
TestTarget{ | |
.os = builtin.Os.macosx, | |
.arch = builtin.Arch.x86_64, | |
.abi = builtin.Abi.gnu, | |
}, | |
TestTarget{ | |
.os = builtin.Os.windows, | |
.arch = builtin.Arch.x86_64, | |
.abi = builtin.Abi.msvc, | |
}, | |
}; |
You can manually run the behavior tests for the native target with ./zig test ../test/stage1/behavior.zig
and the std lib tests with ./zig test ../std/std.zig --override-std-dir ../std
.
Do we have any way to run CI tests for DragonFly BSD?
this last zig-master issue was caused by copying an old native_libc.txt file from zig-0.4.0, and easily fixed
Let's try to get it working with master. After fixing up code from my review comments, let me know what happens when you delete native_libc.txt and then try to link against libc.
Note: the tests with --single-threaded fail with lld error: TLS attribute mismatch: errno. Which was defined with threadlocal
I pushed a fix for this to master: bd9c629
src/link.cpp
Outdated
@@ -872,7 +872,7 @@ static const char *getLDMOption(const ZigTarget *t) { | |||
return "elf32_x86_64"; | |||
} | |||
// Any target elf will use the freebsd osabi if suffixed with "_fbsd". | |||
if (t->os == OsFreeBSD) { | |||
if (t->os == OsFreeBSD || t->os == OsDragonFly) { | |||
return "elf_x86_64_fbsd"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DragonFly BSD uses the "fbsd" suffix? Can you confirm this?
src/link.cpp
Outdated
"-L/raven/share/raven/toolchain/gcc8/lib"); | ||
// prefer .a over .so | ||
// fixes: lld: warning: found local symbol '__thread_locale' in global part of symbol table in file /raven/share/raven/sysroot/DragonFly/usr/lib/libc.so | ||
lj->args.append("/raven/share/raven/sysroot/DragonFly/usr/lib/libc.a"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it should be the crt_dir
.
std/c/dragonfly.zig
Outdated
pub extern "c" fn clock_getres(clk_id: c_ulong, tp: *timespec) c_int; | ||
|
||
/// Renamed from `kevent` to `Kevent` to avoid conflict with function name. | ||
pub const Kevent = extern struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you come up with the extern function prototypes and the struct definitions in this file?
std/os/dragonfly.zig
Outdated
@@ -0,0 +1,845 @@ | |||
const builtin = @import("builtin"); | |||
|
|||
pub use @import("dragonfly/errno.zig"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you copy this file from FreeBSD? I'm worried that there are hard-to-catch bugs in here if there is anything different.
std/os/dragonfly/errno.zig
Outdated
@@ -0,0 +1,120 @@ | |||
pub const EPERM = 1; // Operation not permitted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the source of information for this file?
Testing functions can give runtime success/error -- even when they have untaken paths with comptime errors This issue first occured because I thought I am still stuck on this. Wrapping the
Other than this issue, it's mostly mutex/atomic/event tests failing. I got mutex.zig code compiling but core dumping, so I stripped it out Thanks for all your help. Let me note these below issues too, so that I've covered everything I've now added dragonfly to test.zig. Many tests run. But the command Also running the command
c/dragonfly.zig is now auto-generated with translate-c, and os/dragonfly.zig just wraps those values When translating
So I just change that line to:
Other than that std/c/dragonfly.zig is essentially a direct translation I just rebased to HEAD, and got this. Let me push it anyway.... :(
|
@Hejsil can you take a look at this? I think this may be related to 6b10f03. |
Ok, maybe not real quick. I'm a little busy and don't have a computer on hand with a building version of Zig master. |
@tse-gratis Should be fixed with c051904. Sorry for the inconvenience. |
Thanks @Hejsil for making this possible Quick overview:
Of those 19 skipped; they are the same as the previous commit: dirent.reclen in os.zig, mutex.zig, etc |
std/c/dragonfly.zig
Outdated
pub const CTLTYPE_S16 = 13; | ||
pub const CTLTYPE_NODE = 1; | ||
pub const __PTRDIFF_FMTi__ = c"li"; | ||
pub const __SSE4_1__ = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is about your computer; not from dragonfly itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😳 Thanks @daurnimator 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this better?
std/c/dragonfly.zig
Outdated
pub const SF_XLINK = 16777216; | ||
pub const S_IFSOCK = 49152; | ||
pub const __GCC_ATOMIC_LLONG_LOCK_FREE = 2; | ||
pub const __clang_version__ = c"8.0.0 (tags/RELEASE_800/final)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Things starting with __
are generally from the compiler.
std/c/dragonfly.zig
Outdated
pub const T_ALIGNFLT = 14; | ||
pub const TIMER_ABSTIME = 1; | ||
pub const SIG_BLOCK = 1; | ||
pub const SIGCHLD = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should at least be sorted/grouped
I'm sorry for the timing but that big branch I just merged moved a lot of standard library stuff around and you'll have to do some rebasing. It might make sense to look at a diff of your changes and then manually re-apply them to master. The good news is that it's highly likely the changes you have to make to support dragonfly will be much fewer. |
I'm kind of running out of steam This branch is still essentially waiting for review, but it has gained some new issues
Other than those small things 😉 I realize the file c_to_zig.sh needs removing, and the commits need squashing down |
The azure ci fails with So not really the code in this branch And thanks, the rebasing changes were simpler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your rebase got messed up and reverted a bunch of stuff from master branch.
src/all_types.hpp
Outdated
@@ -1857,7 +1857,7 @@ struct CodeGen { | |||
BuildMode build_mode; | |||
OutType out_type; | |||
const ZigTarget *zig_target; | |||
TargetSubsystem subsystem; // careful using this directly; see detect_subsystem | |||
TargetSubsystem subsystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your rebase resolved some conflicts the wrong way
src-self-hosted/translate_c.zig
Outdated
@@ -6,7 +6,7 @@ const builtin = @import("builtin"); | |||
const assert = std.debug.assert; | |||
const ast = std.zig.ast; | |||
const Token = std.zig.Token; | |||
usingnamespace @import("clang.zig"); | |||
use @import("clang.zig"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usingnamespace
is correct. You need to rebase against master again; this changed yesterday. See #2014
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I swapped these back because they caused the ci to fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ci is green for master branch which has the usingnamespace
syntax
src/analyze.cpp
Outdated
@@ -2722,10 +2722,12 @@ void add_fn_export(CodeGen *g, ZigFn *fn_table_entry, Buf *symbol_name, GlobalLi | |||
if (ccc) { | |||
if (buf_eql_str(symbol_name, "main") && g->libc_link_lib != nullptr) { | |||
g->have_c_main = true; | |||
g->subsystem = g->subsystem == TargetSubsystemAuto ? TargetSubsystemConsole : g->subsystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an accident from the rebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay thanks. I will need to try again
src/analyze.cpp
Outdated
} else if (buf_eql_str(symbol_name, "WinMain") && | ||
g->zig_target->os == OsWindows) | ||
{ | ||
g->have_winmain = true; | ||
g->subsystem = g->subsystem == TargetSubsystemAuto ? TargetSubsystemWindows : g->subsystem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an accident from the rebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes. Hopefully the latest commit 80a0499 is better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more change and then I think this is ready to merge.
std/c/dragonfly.zig
Outdated
return &errno; | ||
} | ||
pub const __sighandler_t = extern fn(c_int) void; | ||
// auto import |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto import brought in some undesirable stuff, such as NULL
and probably more that isn't DragonFly OS specific. Due to zig's lazy analysis however this is mostly fine, as unused things are generally harmless.
Types and constants should be moved to std/os/bits/dragonfly.zig. The extern "c" functions should stay.
Oh, I also wanted to ask, why are all those tests disabled? What were the errors you were getting? |
= Minor notes = itimerspec conflicted with definition in os/linux.zig, so has been removed from os/bits/dragonfly.zig Just to reflect the layout of freebsd: I kept = Std test issues avoided with SkipZigTest =
|
Why was that a problem? |
I had a look at this today. There is still more to do before it's merge-ready:
|
I'm closing this since there has been no progress for 1 month. @tse-gratis or anyone else who wants to continue working on this, please feel free to open a new pull request. In the future I would suggest smaller, incremental patches, taking advantage of Zig's lazy analysis. Get the stage0 compiler building. Pull Request. Get Hello World working. Pull Request. Then move on to more advanced test cases. Doing it all at once can be overwhelming. |
Hiya,
This has the initial compiler working, so zig can compile executables on DragonFlyBSD. I'm intending it for ravenports, which is an os-independent port manager
The major issues (at least as far as I can see) are that
src/link.cpp
adds a few hardwired paths. Not sure of the best way around thatbuild --build-file ../build.zig test-behavior
for zig-0.4.0 gives multipleThis is because
x86_64-linux-gnu
is getting passed into main.cpp at each testDoing 3. for zig-master replaces zig-cache/native_libc.txt with the system paths (overwriting the raven paths that were used to build zig), and produces
I haven't debugged this issue, but at least simple build-exe works
Edit: this last zig-master issue was caused by copying an old native_libc.txt file from zig-0.4.0, and easily fixed