Skip to content

compiler_rt: add __clzdi2 and __clzti2 #9578

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

Merged
merged 1 commit into from
Sep 1, 2021
Merged

Conversation

matu3ba
Copy link
Contributor

@matu3ba matu3ba commented Aug 17, 2021

  • structure derived from shift.zig
  • rename clzsi2.zig to count0bits.zig
  • test cases derived from clzsi2_test.zig

See #1290

@matu3ba
Copy link
Contributor Author

matu3ba commented Aug 17, 2021

Looks like the i386 can not handle word length of 128bit (i128). The manual only mentions quadword, which are 64 bits.

I will create a README.md and word.zig shortly explaining what word size etc means and why we need to do the word dance wrt endianess and maximum supported word size.

Personally I think having optimised versions by 1. (maximum) word size and 2. endianess would make sense, because zig may ship no LLVM and hence no peephole optimiser. However this complicates code and may introduce more maintenance, so its a tradeoff. Little endian algorithms are always simpler (+ faster), if architecture supports according word size.

Test [127/139] compiler_rt.clzti2_test.test "compiler-rt-i386-linux-none-Debug-bare-mult... Segmentation fault at address 0xc /home/vsts/work/1/s/lib/std/special/compiler_rt/clzti2.zig:25:5: 0x8b81b7 in compiler_rt.clzti2.__clzti2_generic (test) return n - @bitcast(i128, x); ^ /home/vsts/work/1/s/lib/std/special/compiler_rt/clzti2_test.zig:15:30: 0x9683d7 in compiler_rt.clzti2_test.test__clzti2 (test) var result = actualClzti2(x); ^ /home/vsts/work/1/s/lib/std/special/compiler_rt/clzti2_test.zig:20:21: 0x8e85f9 in compiler_rt.clzti2_test.test "compiler-rt-i386-linux-none-Debug-bare-multi clzti2" (test) try test__clzti2(0x00800000_00000000_00000000_00000000, 8); ^ /home/vsts/work/1/s/lib/std/special/test_runner.zig:76:28: 0x9430a5 in std.special.main (test) } else test_fn.func(); ^ /home/vsts/work/1/s/lib/std/start.zig:505:22: 0x93996a in std.start.callMain (test) root.main(); ^ /home/vsts/work/1/s/lib/std/start.zig:457:12: 0x920fa2 in std.start.callMainWithArgs (test) return @call(.{ .modifier = .always_inline }, callMain, .{}); ^ /home/vsts/work/1/s/lib/std/start.zig:371:17: 0x91d002 in std.start.posixCallMainAndExit (test) std.os.exit(@call(.{ .modifier = .always_inline }, callMainWithArgs, .{ argc, argv, envp })); ^ /home/vsts/work/1/s/lib/std/start.zig:284:5: 0x91cead in std.start._start (test) @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{}); ^ qemu: uncaught target signal 6 (Aborted) - core dumped error: the following test command crashed: qemu-i386 /home/vsts/work/1/s/zig-cache/o/043ef600021677d5b227f1f9a5e165be/test /home/vsts/work/1/s/build/release/bin/zig The following command exited with error code 1: /home/vsts/work/1/s/build/release/bin/zig test /home/vsts/work/1/s/lib/std/special/compiler_rt.zig --test-name-prefix compiler-rt-i386-linux-none-Debug-bare-multi --cache-dir /home/vsts/work/1/s/zig-cache --global-cache-dir /home/vsts/.cache/zig --name test -target i386-linux-none --test-cmd qemu-i386 --test-cmd-bin -I /home/vsts/work/1/s/test --zig-lib-dir /home/vsts/work/1/s/lib error: the following build command failed with exit code 1: /home/vsts/work/1/s/zig-cache/o/c7fc03ac1f14311b29096b9002efd9df/build /home/vsts/work/1/s/build/release/bin/zig /home/vsts/work/1/s /home/vsts/work/1/s/zig-cache /home/vsts/.cache/zig test-toolchain -Denable-qemu -Denable-wasmtime ##[error]Bash exited with code '1'.

@matu3ba matu3ba marked this pull request as draft August 17, 2021 23:14
@matu3ba matu3ba changed the title Compiler rt: __clzdi2 and __clzti2 compiler_rt: add __clzdi2 and __clzti2 Aug 18, 2021
@matu3ba matu3ba force-pushed the compiler_rt branch 4 times, most recently from a16c5d3 to 6db20b5 Compare August 19, 2021 09:33
@matu3ba matu3ba marked this pull request as ready for review August 19, 2021 11:15
@Vexu Vexu added the standard library This issue involves writing Zig code for the standard library. label Aug 21, 2021
@matu3ba matu3ba marked this pull request as draft August 22, 2021 09:19
@matu3ba matu3ba force-pushed the compiler_rt branch 3 times, most recently from b4d673f to 9fbc6e9 Compare August 25, 2021 16:10
@matu3ba matu3ba marked this pull request as ready for review August 25, 2021 20:19
@andrewrk
Copy link
Member

andrewrk commented Sep 1, 2021

Thanks @matu3ba - this looks ready to merge. However-

LLVM 12.x regressed on MIPS, so big endian can not be tested in CI and thus was omitted.

Today I pushed d0f0482 and fixed the CI tarballs with 75263e1. Mind rebasing against latest master with those MIPS test enabled?

- structure derived from shift.zig
- rename clzsi2.zig to count0bits.zig
- test cases derived from clzsi2_test.zig

See ziglang#1290
@matu3ba
Copy link
Contributor Author

matu3ba commented Sep 1, 2021

@andrewrk Sweetly enough it works without any changes and the ugly big endian spaghetti code of LLVM. Just hope debug semantics match release ones.

fn __clzsi2_generic(a: i32) callconv(.C) i32 {
@setRuntimeSafety(builtin.is_test);
// clz - count leading zeroes
// - clzXi2_generic for little endian
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change the text in a followup PR for parity or ctz.

Copy link
Contributor Author

@matu3ba matu3ba left a comment

Choose a reason for hiding this comment

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

This one comment should be changed, which I will do in a follow-up PR.

@andrewrk andrewrk merged commit 81e2034 into ziglang:master Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants