The Wayback Machine - https://web.archive.org/web/20240516130513/https://github.com/odin-lang/Odin/pull/3439
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

NetBSD support #3439

Merged
merged 27 commits into from May 15, 2024
Merged

NetBSD support #3439

merged 27 commits into from May 15, 2024

Conversation

andreas-jonsson
Copy link
Sponsor Contributor

@andreas-jonsson andreas-jonsson commented Apr 16, 2024

This is my attempt at an initial port of Odin to NetBSD. It targets NetBSD 10.0 (AMD64) with Clang/LLVM 17 from the pkgsrc wip section pkgsrc. If you don't want to taint your system with WIP libraries you can build LLVM using the mksandbox tool.

The current goal (at the moment) is to have the compiler fully working on NetBSD as well as enough of the standard library to run demo.odin is to have the NetBSD port be on par with the other BSD's.

Right now the compiler seems happy and the demo is running, but there are concurrency issues in the runtime. This needs more investigation. One possible issue could be the thread id, that I do not know how to get a hold of atm and I'm currently investigating the issue. Another culprit could be the futex implementation that is pretty much the same as on Haiku.

There is also room for some more cleanup and possible joining or refactoring some code that could be shared with other BSDs.
Generally this port follow the patterns outlined by the other BSD's.

@gingerBill gingerBill marked this pull request as draft April 16, 2024 13:21
Call pthread_setname_np with the correct number of arguments on NetBSD.
Hardlink libc functions to the correct version on NetBSD 10 since we do not use the micro-magic from C.
Updated core with some path related functions and did some minor code cleanup.
Most of the standard library function is just a matter of copy what is there for the other BSDs.
@andreas-jonsson andreas-jonsson changed the title [WIP] NetBSD support NetBSD support Apr 18, 2024
Fixed broken thread policy causing deadlocks.
Just selecting the same codepath as other BSD's for the most part.
@andreas-jonsson
Copy link
Sponsor Contributor Author

I think we are at a pretty good state right now. Would love to get some feedback.

@andreas-jonsson andreas-jonsson marked this pull request as ready for review April 19, 2024 10:34
@Yawning
Copy link
Contributor

Yawning commented Apr 19, 2024

Add the appropriate build tag to core/crypto/rand_bsd.odin.

Added build tag to rand_bsd.odin and fixed build warning.
I realize we should use PHYSMEM64 on NetBSD. So we can not share code with FreeBSD.
@Yawning
Copy link
Contributor

Yawning commented Apr 22, 2024

Ah sorry, you also need to exclude netbsd in the generic core/crypto/rand_generic.odin. The tests under tests/core will fail to compile unless you do.

@laytan
Copy link
Sponsor Contributor

laytan commented Apr 24, 2024

Are you sure netbsd doesn't have native futexes?

@andreas-jonsson
Copy link
Sponsor Contributor Author

andreas-jonsson commented Apr 25, 2024

Are you sure netbsd doesn't have native futexes?

I don't think so. :(
I was not able to find any documentation about it at least. Also, was not able to find any headers hinting of it's being supported.

(It could perhaps be called something else on NetBSD? I noticed it was not easy to find on FreeBSD either.)

@andreas-jonsson
Copy link
Sponsor Contributor Author

I think I can rewrite the futex implementation using raw syscalls. Will give it a go.

@andreas-jonsson
Copy link
Sponsor Contributor Author

Native futex support is in. Plz let me know if you have any other feedback.

@Yawning
Copy link
Contributor

Yawning commented Apr 25, 2024

Native futex support is in. Plz let me know if you have any other feedback.

I looked into this a bit and came across this thread, which is concerning. Though the followup by thorpej seems to indicate that it may be usable in certain cases.

otoh Mesa doesn't use futexes on NetBSD when I peeked at the sourcecode, so I don't know if/how the followup progressed.

https://man.netbsd.org/_lwp_park.2 and co might be the "right" thing to do, but NetBSD appears to be unique among the BSDs here.

@andreas-jonsson
Copy link
Sponsor Contributor Author

Hm.. yea.
That is an interesting read. I hope the NetBSD folks fix it because it is convenient for porting purposes to follow Linux here.

As for the current state of things I have hammered this code quite a bit and I have not found any issues. (That is not to say there is none.) But it could also be that this works well for Odin since the use of the API is quite simplistic, and it could be that most of the issues the Mesa team is experiencing comes from using more "complicated" patterns. Perhaps there use of futexes are not process private for example?

I would prefer keeping the code as is and do wider testing. If we see any problems it is only a few lines of code to switch to the _lwp_park API. Most of the work would just be to read up on the API documentation.

If this work, the codebase also gets cleaner because most code is just shared with Linux. :)

@andreas-jonsson
Copy link
Sponsor Contributor Author

There is now a working CI for Odin on NetBSD in a separate PR here #3482
Result can be seen here: https://github.com/andreas-jonsson/Odin/actions/runs/8925922741/job/24515753510

Copy link
Sponsor Contributor

@laytan laytan left a comment

Choose a reason for hiding this comment

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

Looks great in general!

core/sys/unix/pthread_netbsd.odin Outdated Show resolved Hide resolved
src/build_settings.cpp Outdated Show resolved Hide resolved
src/build_settings.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Yawning Yawning left a comment

Choose a reason for hiding this comment

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

Mostly nits, free feel to ignore.

core/sys/info/cpu_intel.odin Show resolved Hide resolved
core/crypto/rand_bsd.odin Show resolved Hide resolved
core/os/os_netbsd.odin Show resolved Hide resolved
core/os/os_netbsd.odin Outdated Show resolved Hide resolved
@gingerBill gingerBill merged commit f9fd8f0 into odin-lang:master May 15, 2024
4 checks passed
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.

None yet

4 participants