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

Zig support improvement #602

Closed
11 tasks done
ivmai opened this issue Dec 25, 2023 · 19 comments
Closed
11 tasks done

Zig support improvement #602

ivmai opened this issue Dec 25, 2023 · 19 comments

Comments

@ivmai
Copy link
Owner

ivmai commented Dec 25, 2023

Continuation of #557 and PR #598.

What to improve in build.zig:

  • resolve TODO item: specify PACKAGE_VERSION and LIB*_VER_INFO, convert VER_INFO values to [SO]VERSION ones (moved to Specify soversion for shared libs built by Zig #638)
  • resolve TODO item: support enable_cplusplus
  • resolve TODO item: support enable_throw_bad_alloc_library
  • resolve TODO item: support build_cord
  • change test names to have "test" suffix as produced by cmake
  • support MacOS targets
  • remove -Dbuild_tests options

What to improve in GH Actions:

  • update to zig v0.12 final (when released)
  • support cross-compile for MacOS
  • build and test natively on MacOS
  • support cross-compile for other tier 1 targets
  • test with -Denable_cplusplus
ivmai added a commit that referenced this issue Dec 31, 2023
@ivmai
Copy link
Owner Author

ivmai commented Jan 9, 2024

@plajjan It would be very convenient to support CFLAGS_EXTRA option, could you please add it?
See CMakeLists.txt and Makefile.direct for the reference. (I'm not sure we need to separate the arguments in CFLAGS_EXTRA.)

@plajjan
Copy link
Contributor

plajjan commented Jan 10, 2024

With "replace -Denable_threads=false to -fsingle-threaded", do you mean we should replace the user visible input? I don't think that is possible. I think the idea with zig build is that you don't just let users pass compiler flags straight to the compiler but rather expose them as project specific options (the -D options) and then set compiler flags based on that.

If on the other hand you mean that we should set -fsingle-threaded based on -Denable_threads=false, that should certainly be possible. Zig compilation objects, i.e. libraries and executable, have a single_threaded option on them, that we should probably set. I think that implies -fsingle-threaded down the line.

@plajjan
Copy link
Contributor

plajjan commented Jan 10, 2024

@ivmai yes, I can add a simple CFLAGS_EXTRA option that just passes through its value to the compiler. I don't see any added value in parsing it to separate arguments etc, so straight pass-through it is! :)

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

With "replace -Denable_threads=false to -fsingle-threaded", do you mean we should replace the user visible input? I don't think that is possible. I think the idea with zig build is that you don't just let users pass compiler flags straight to the compiler but rather expose them as project specific options (the -D options) and then set compiler flags based on that.

If on the other hand you mean that we should set -fsingle-threaded based on -Denable_threads=false, that should certainly be possible. Zig compilation objects, i.e. libraries and executable, have a single_threaded option on them, that we should probably set. I think that implies -fsingle-threaded down the line.

I mean expose change build.zig to let client build single-threaded library using -fsingle-threaded option (more nature way in zig world if I understand it correctly) instead of -Denable_threads=false option. I.e.:
zig build -fsingle-threaded instead of zig build -Denable_threads=false.
If this possible. Not a big deal, of course.

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

Similarly, it looks like -Dbuild_tests option is redundant (to build and run the tests we need to pass both this option and test). I think it worth to remove build_tests option.

@plajjan
Copy link
Contributor

plajjan commented Jan 10, 2024

@ivmai I'm no authoritative zig expert but as far as I know this is not possible. zig build-exe supports -fsingle-threaded but this is for compiling a single zig module / .c into an executable. It is similar to invoking cc directly. With zig build we use zig as a build system and not as a direct compiler, so we are expected to control things like single-threaded through code in build.zig and based on that pass down parameters to the compiler.

Just testing zig build -fsingle-threaded is invalid...

kll@Boxy:~/terastream/acton-deps/bdwgc$ ~/kod/zig/zig-linux-x86_64-0.12.0-dev.2105+60094cc3f/zig build -fsingle-threaded
Unrecognized argument: -fsingle-threaded
...

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

Okay (about -fsingle-threaded). I am removing it from issue description.

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

@plajjan Could we make test names as given in cmake script? (hugetest, subthreadcreatetest, etc.)

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

@ivmai yes, I can add a simple CFLAGS_EXTRA option that just passes through its value to the compiler. I don't see any added value in parsing it to separate arguments etc, so straight pass-through it is! :)

It seems that cmake's separate_arguments() is needed, see #607 (comment)

@ivmai
Copy link
Owner Author

ivmai commented Jan 10, 2024

Similarly, it looks like -Dbuild_tests option is redundant (to build and run the tests we need to pass both this option and test). I think it worth to remove build_tests option.

@plajjan I will remove this option. If you agree it is redundant.

@ivmai
Copy link
Owner Author

ivmai commented Jan 11, 2024

@plajjan Could we make test names as given in cmake script? (hugetest, subthreadcreatetest, etc.)

I'll do it.

ivmai added a commit that referenced this issue Jan 11, 2024
Issue #602 (bdwgc).

The tests are built only if "test" step is requested (similar to
Makefile), thus no need for the build_tests option.

* build.zig (build_tests): Remove option (const); assume it is always
true; add comment.
ivmai added a commit that referenced this issue Jan 11, 2024
Issue #602 (bdwgc).

* README.md (Zig): Update sample zig build command to run the tests
as well.
ivmai added a commit that referenced this issue Jan 11, 2024
Issue #602 (bdwgc).

* build.zig (build): Pass specific filename to each addTest() call.
* build.zig (addTest): Add filename argument instead of computing it
from testname.
ivmai added a commit that referenced this issue Jan 12, 2024
Issue #602 (bdwgc).

If MISSING_MACH_O_GETSECT_H is defined (manually), then do not include
mach-o/getsect.h file.

* dyn_load.c [DARWIN]: Do not include getsect.h if
MISSING_MACH_O_GETSECT_H.
* dyn_load.c [DARWIN && MISSING_MACH_O_GETSECT_H] (getsectiondata):
Declare extern function.
* dyn_load.c [DARWIN] (GC_MACH_HEADER): Move definition upper (to be
before getsectiondata() declaration).
ivmai added a commit that referenced this issue Jan 12, 2024
@kassane
Copy link
Contributor

kassane commented Apr 13, 2024

Taking advantage of the Zig toolchain topic.
I haven't seen any reference about libc++ yet as it is statically linked by default in Zig on all LLVM targets.

What is the level of support between C++ ABI?

@plajjan
Copy link
Contributor

plajjan commented Apr 14, 2024

@kassane when I built C++ things with Zig I have had to add .linkLibCpp() to my build.zig to get it working, doesn't that mean libc++ is not linked in per default?

@kassane
Copy link
Contributor

kassane commented Apr 14, 2024

Yes. By adding -lc++ or .linkLibCpp() it is linked by default to llvm-libcxx (static only).

So, bdwgc supports it has no restriction in C++ABI. Right?
If there was any favoring the GNU ABI (libstdc++) it might be problematic.

@ivmai
Copy link
Owner Author

ivmai commented Apr 15, 2024

So, bdwgc supports it has no restriction in C++ABI?

Right. It should not, at least.

@ivmai
Copy link
Owner Author

ivmai commented Apr 16, 2024

It would be good to add building of libgccpp.so, libgctba.so and cpptest to build.zig (at least to have a working sample of C++ support).

@kassane
Copy link
Contributor

kassane commented Apr 17, 2024

also, add msvc support using zig-build.

// zig v0.12.0 - for C++ build on msvc or gnu

// https://github.com/ziglang/zig/issues/5312
if (exe.rootModuleTarget().abi != .msvc)
    exe.linkLibCpp() // libc++ + libunwind [static]
else
    exe.linkLibC(); // search VSCrt + winSDK

@kassane
Copy link
Contributor

kassane commented Apr 18, 2024

It would be good to add building of libgccpp.so, libgctba.so and cpptest to build.zig (at least to have a working sample of C++ support).

My sample: kassane/bdwgc-d@9944b65
CI: https://github.com/kassane/bdwgc-d/actions/runs/8741234879

New (mix C++/D)
Sample: kassane/bdwgc-d@5bed7c8
CI: https://github.com/kassane/bdwgc-d/actions/runs/8742595019

ivmai added a commit that referenced this issue Apr 24, 2024
Issue #602 (bdwgc).

The final version of zig v0.12.0 has been released recently.

* build.zig (zig_min_required_version): Update to "0.12.0"; remove TODO
item.
ivmai added a commit that referenced this issue Apr 27, 2024
(refactoring)

Issue #602 (bdwgc).

* build.zig (build): Rename lib variable to gc one.
* build.zig (addTest): Rename lib argument to gc one.
ivmai added a commit that referenced this issue Apr 27, 2024
Issue #602 (bdwgc).

This commit adds support of build_cord option to Zig script (to match
that of CMake script).

* build.zig (build): Add build_cord const (option); define cord
variable.
* build.zig [build_cord] (build): Define cord_src_files variable;
specify source files and dependency library for cord; install cord
artifact; call addTestExt() for cordtest; add TODO item about de.
* build.zig [install_headers && build_cord] (build): Install cord
headers.
* build.zig (addTestExt): New function (move most code from addTest
except for added lib2 argument).
* build.zig (addTest): Redirect to addTestExt().
@ivmai
Copy link
Owner Author

ivmai commented Apr 27, 2024

Colleagues, I've added build of cord library (7201f3f) - the code is work, but maybe you could suggest me tips to improve the code if any (cordtest request linking against 2 libs, so I've added addTestExt to be able to pass 2 libs for a test).

ivmai added a commit that referenced this issue Apr 27, 2024
…t arm

(fix of commit 7201f3f)

Issue #602 (bdwgc).

* build.zig [build_cord && enable_werror && !enable_threads
&& (t.abi==.gnueabi || t.abi==.gnueabihf || t.abi==.musleabi
|| t.abi==.musleabihf)] (build): Define AO_DISABLE_GCC_ATOMICS macro;
add TODO item.
@ivmai ivmai closed this as completed Apr 30, 2024
ivmai added a commit that referenced this issue May 3, 2024
(refactoring)

Issue #602 (bdwgc).

* build.zig (build): Use append() instead of appendSlice() for
source_files, flags, gctba_src_files where only 1 element is appended.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants