compiler-rt math functions reorg #11532
Conversation
andrewrk
force-pushed
the
compiler-rt-math
branch
from
dc74f1b to
a3e7b67
Compare
andrewrk
changed the title
| return math.ln(2 * x - 1 / (x + math.sqrt(x * x - 1))); | ||
| return @log(2 * x - 1 / (x + @sqrt(x * x - 1))); |
There was a problem hiding this comment.
math.ln and @log are not equivalent. ln is base e and log is base 10. You would need to divide by @log(e).
There was a problem hiding this comment.
@log does refer to the natural logarithm. See https://ziglang.org/documentation/0.9.1/#log.
There was a problem hiding this comment.
Thanks, I should have checked before commenting. Sorry for that!
There was a problem hiding this comment.
It's still a fair point; we may want to play with the naming to make it less ambiguous. Although we are a bit limited by libc here since we do need to match the symbol names, and libc has log as natural log.
* unify the logic for exporting math functions from compiler-rt,
with the appropriate suffixes and prefixes.
- add all missing f128 and f80 exports. Functions with missing
implementations call other functions and have TODO comments.
- also add f16 functions
* move math functions from freestanding libc to compiler-rt (#7265)
* enable all the f128 and f80 code in the stage2 compiler and behavior
tests (#11161).
* update std lib to use builtins rather than `std.math`.
Updates stage1 to manually lower softfloat operations for all unary floating point operations, extension/truncation, and arithmetic.
Updates stage2 to manually lower softfloat operations for all unary floating point operations and arithmetic. Softfloat support still needs to be added for conversion operators (float<->float and int<->float)
* std.math.snan: fix compilation error. Also make it and nan inline. * LLVM: use a proper enum type for float op instead of enum literal. Also various cleanups. * LLVM: use LLVMBuildVectorSplat for vector splat AIR instruction. - also the bindings had parameter order wrong * LLVM: additionally handle f16 lowering. For now all targets report OK but I think we will need to add some exceptions to this list.
andrewrk
force-pushed
the
compiler-rt-math
branch
from
a3e7b67 to
c4eaff6
Compare
The reason for having `@tan` is that we already have `@sin` and `@cos` because some targets have machine code instructions for them, but in the case that the implementation needs to go into compiler-rt, sin, cos, and tan all share a common dependency which includes a table of data. To avoid duplicating this table of data, we promote tan to become a builtin alongside sin and cos. ZIR: The tag enum is at capacity so this commit moves `field_call_bind_named` to be `extended`. I measured this as one of the least used tags in the zig codebase. Fix libc math suffix for `f32` being wrong in both stage1 and stage2. stage1: add missing libc prefix for float functions.
This is to account for the small differences in math functions of different libcs. For example, if the compiler links against glibc, but the target is musl libc, then these values might be slightly different. Arguably, this is a bug in the compiler because comptime should emulate the target, including rounding errors in libc math functions. However that behavior is not what this particular test is intended to cover.
Weak aliases don't work on Windows, so we avoid exporting the `l` alias on this platform for functions we know will collide.
This was leftover from testing std.math.sin
This is a new test added in this branch but it is not yet passing for i386-windows with the stage1 compiler.
These are only as accurate as f64 even for f128 comptime functions. This is OK for now; improvements will come with the launch of self-hosted (#89) and enhancements to compiler-rt implementations.
This avoids the optimizer turning sincos into a recursive call to itself.
Before this change, struct {f80, f80} targeting i386-windows-msvc lowers
to
```llvm
%"std.testing.struct:78:61.6" = type { x86_fp80, [6 x i8], x86_fp80, [6 x i8] }
```
which has an incorrect ABI size of 40. After this change, the struct
lowers to
```llvm
%"std.testing.struct:78:61.6" = type { x86_fp80, [4 x i8], x86_fp80, [4 x i8] }
```
which has the correct ABI size of 32, and properly aligns the second
field to 16 bytes.
The other place that calculates field padding (lowering of constant
values in codegen.cpp) already correctly calls LLVMABISizeOfType
rather than LLVMStoreSizeOfType.
This fixes the compiler-rt tests for i386-windows in this branch.
andrewrk
added
the
breaking
|
Labeling as "breaking" because this removes |
| @@ -8055,6 +8188,26 @@ fn backendSupportsF80(target: std.Target) bool { | |||
| }; | |||
| } | |||
|
|
|||
| /// This function returns true if we expect LLVM to lower f16 correctly | |||
| /// and false if we expect LLVM to crash if it counters an f16 type or | |||
| /// if it produces miscompilations. | |||
There was a problem hiding this comment.
This sounds like stuff that would be very useful, if the user could query this.
Or what do you think? (Same argument holds for other codegen backends)
There was a problem hiding this comment.
No, it is best if the user does not check these conditions. Technically, they already can, because they can put this logic into their application code and check for @import("builtin").zig_backend being one of the LLVM backends. However, see the doc comment on the CompilerBackend enum for best practices related to this.
| } | ||
|
|
||
| /// LLVM does not support all relevant intrinsics for all targets, so we | ||
| /// may need to manually generate a libc call |
There was a problem hiding this comment.
same idea: offering the opportunity to query from build.zig if intrinsics are used sounds useful.
|
Would you be opposed to having backward compatible inline fn's included in math? Code doing complex math is already difficult enough to follow as is... remembering which is builtin vs in math just adds unnecessary friction (and makes the code ugly imo). |
|
Not opposed |
In the math builtin functions documentation, remove the link to issue ziglang#4026, since it was closed by ziglang#11532.
with the appropriate suffixes and prefixes.
implementations call other functions and have TODO comments.
tests
std.math.tanis moved to compiler-rt to match@sinand@cos, because all three implementations share a common dependency.Merge checklist:
l,q, orxsuffixes as appropriate #11161@tanbuiltin.