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

add no_std support #1

Merged
merged 1 commit into from Sep 19, 2018
Merged

add no_std support #1

merged 1 commit into from Sep 19, 2018

Conversation

ghost
Copy link

@ghost ghost commented Aug 26, 2018

@ghost ghost mentioned this pull request Aug 26, 2018
@ghost
Copy link
Author

ghost commented Sep 14, 2018

Ping

std = []

[target.'cfg(not(feature = "std"))'.dependencies]
libm = "0.1.2"
Copy link
Owner

@eira-fransham eira-fransham Sep 17, 2018

Choose a reason for hiding this comment

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

Is this only for fract? Their implementation of trunc (which fract is a simple transformation of) seems kinda suspect. Specifically it forces a volatile read which can result in poor performance since now AIUI it must be spilled to the stack instead of kept solely in registers. Look at their implementation of force_eval to see the volatile read. Might be worth putting in a PR to them. It looks OK from a behaviour perspective though so we can deal with that later if we want to.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is only for fract.

The force_eval is copied from musl: https://git.musl-libc.org/cgit/musl/tree/src/math/trunc.c

I honestly haven't looked at performance. I want this mostly for wasmi.
That being said, libm is a non-public dependency, so it can be switched out any time.

Copy link
Owner

Choose a reason for hiding this comment

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

Fair enough! If it's good enough for musl, it's good enough for me

@eira-fransham eira-fransham merged commit b7dd399 into eira-fransham:master Sep 19, 2018
@ghost
Copy link
Author

ghost commented Sep 19, 2018

Could you release this, please?
So that wasmi can depend on it.

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.

1 participant