Skip to content

Add sincosf function #7267

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 2 commits into from
Dec 23, 2020
Merged

Add sincosf function #7267

merged 2 commits into from
Dec 23, 2020

Conversation

daurnimator
Copy link
Contributor

Fixes #7260

@MasterQ32 I have no idea how to test this, you didn't leave a reproducer.

@ikskuh
Copy link
Contributor

ikskuh commented Dec 1, 2020

@MasterQ32 I have no idea how to test this, you didn't leave a reproducer.

Me neither. It just happens somewhere in > 5000 LOC where code uses a lot of sin and cos

@LemonBoy
Copy link
Contributor

LemonBoy commented Dec 1, 2020

Here's the man page, some tests are needed to make sure this implementation matches the libc one (wrt nan/infinity inputs, the errno part should be honored too?).

Adding the sincos variant is trivial.

@daurnimator
Copy link
Contributor Author

the errno part should be honored too?

great question. At first I thought so, but then I saw that e.g. sinf doesn't....
What is the correct behaviour here?
The fact that LLVM optimises to things that require errno doesn't sit well with me.

@LemonBoy
Copy link
Contributor

LemonBoy commented Dec 1, 2020

The fact that LLVM optimises to things that require errno doesn't sit well with me.

Calm your tits, LLVM is smart. The optimization kicks in iff sin/cos are marked with readnone meaning they don't touch errno at all.
Check out this gcc ticket for more info, the errno clause is specified in the sacred POSIX texts.

@LemonBoy
Copy link
Contributor

LemonBoy commented Dec 1, 2020

Reproducer:

const std = @import("std");

extern var foo: f32;

pub fn main() !void {
    var y = @sin(foo);
    var z = @cos(foo);
    std.mem.doNotOptimizeAway(y);
    std.mem.doNotOptimizeAway(z);
}

Compile with: zig build-exe -target native-linux-gnu -OReleaseFast

@daurnimator
Copy link
Contributor Author

Tested with

const std = @import("std");

export fn myfunc(foo: f32) f32 {
    var y = @sin(foo);
    var z = @cos(foo);
    return y + z;
}
zig build-lib -dynamic -target native-linux-gnu -OReleaseFast sincosf.zig
objdump --disassemble libsincosf.so

and looks correct.... will re-kick CI because error message is gone now.

@daurnimator daurnimator reopened this Dec 13, 2020
@daurnimator daurnimator marked this pull request as ready for review December 14, 2020 04:48
@Vexu Vexu merged commit 53a8e73 into ziglang:master Dec 23, 2020
aarvay pushed a commit to aarvay/zig that referenced this pull request Jan 4, 2021
* Add sincosf function

* also add sincos

Co-authored-by: Veikka Tuominen <[email protected]>
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.

Missing symbol in compiler_rt in release modes
4 participants