1

Faster float conversion operations by m-ou-se · Pull Request #464 · rust-lang/co...

 1 year ago
source link: https://github.com/rust-lang/compiler-builtins/pull/464
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Faster float conversion operations #464

Conversation

Member

@m-ou-se m-ou-se commented 13 days ago

This supersedes #370

This includes #463

Member

Author

m-ou-se commented 13 days ago

Currently, compiler_builtins contains two cases where a builtin is implemented by the native conversion on x86_64:

I'm a bit confused by that. If i as f32 results in native code and not a call to compiler-builtins, then there doesn't seem to be much reason to optimize the version in compiler-builtins.

The version in this PR does not have those special cases. I can put them back if someone thinks they are actually useful.

Contributor

bjorn3 commented 13 days ago

I'm a bit confused by that. If i as f32 results in native code and not a call to compiler-builtins, then there doesn't seem to be much reason to optimize the version in compiler-builtins.

as f32 results in an intrinsic call on platforms that don't have a native version of this.

The version in this PR does not have those special cases. I can put them back if someone thinks they are actually useful.

I think it should be put back.

Member

Author

m-ou-se commented 13 days ago •

edited

as f32 results in an intrinsic call on platforms that don't have a native version of this.

Yes.. That's what I said. The special case is for a platform that does have a native version, which means this function wouldn't be called when someone does as f32.

I think it should be put back.

Why? When would it be used on x86_64?

Contributor

bjorn3 commented 13 days ago

Looks like it is also used in case of +soft-float. I guess that special case wouldn't be valid then though.

Member

@Amanieu Amanieu left a comment

LGTM apart from a few style nits.

src/float/conv.rs

Outdated Show resolved

pub extern "C" fn __floatdidf(i: i64) -> f64 {

// On x86_64 LLVM will use native instructions for this conversion, we

// can just do it directly

if cfg!(target_arch = "x86_64") {

Removing this is fine, I'm not even sure why this hack was here in the first place. If LLVM generates a native instruction then this function is never called anyways. Except in the case of soft-float, in which case you specifically don't want to use the built-in instruction.

m-ou-se reacted with thumbs up emoji

Amanieu

merged commit 8f89148 into

rust-lang:master 7 days ago

25 checks passed

m-ou-se

deleted the floatconv2 branch

6 days ago

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Amanieu

Assignees

Amanieu

Labels
None yet
Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

None yet

3 participants

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK