Skip to content

Change floating-point predicates to intrinsic calls if possible #1999

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

Closed
2 tasks done
catamorphism opened this issue Mar 16, 2012 · 11 comments
Closed
2 tasks done

Change floating-point predicates to intrinsic calls if possible #1999

catamorphism opened this issue Mar 16, 2012 · 11 comments
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. P-low Low priority

Comments

@catamorphism
Copy link
Contributor

  • See the FIXME just before is_positive, is_negative, and the like in std::f32 and std::f64. I tried, but couldn't figure out how to use a macro defined in a C math library header.
  • Also add the macros is_normal and fpclassify mentioned in the FIXME at the end of the predicates in std::f32.
@graydon
Copy link
Contributor

graydon commented Mar 21, 2012

There is no interop between C preprocessor definitions (such as macros) and anything in rust.

@catamorphism
Copy link
Contributor Author

Updated the title to reflect that.

@emberian
Copy link
Member

FIXME still present, think it's relevant.

@pnkfelix
Copy link
Member

Float::is_normal and Float::classify were added in PR #6301.

@pnkfelix
Copy link
Member

I spent a while looking for a signbit intrinsic in the LLVM source (or anything other intrinsic to abstractly implement is_positive, is_negative). I was not successful.

Have we already dismissed operating directly on the bit-representation itself, the way that (my copy of) the math.h header does, with something like: (f as u32 >> 31) ? Or are we not allowed to assume IEEE 754 representation for the floats in f32.rs and f64.rs? (I'll admit, this might do the wrong thing on NaN or other interesting values; I don't remember exactly what boolean comparison does with NaN. Delving a bit more into that now.)

@huonw
Copy link
Member

huonw commented Sep 20, 2013

Won't f < 0 and f > 0 be optimised to the equivalent of an intrinsic is_positive/is_negative?

@pnkfelix
Copy link
Member

@huonw I cannot tell from your comment whether you looked at the implementations of is_positive / is_negative; they do not solely call f > 0 / f < 0, but also include checks for +0.0 and -0.0

That is what I think one needs a signbit intrinsic to access.

(But then again, maybe what you are saying is that LLVM is much more aggressive than I expect and manages to turn f > 0.0 || (1.0 / f) == infinity into a simple signbit access? Somehow I doubt that.)

@huonw
Copy link
Member

huonw commented Sep 24, 2013

I didn't check either the implementation or how aggressively LLVM optimises; it was a genuine question. (I guess that transmuting to a u32/u64 and checking it by hand would be too platform sensitive to be acceptable.)

@pnkfelix
Copy link
Member

(I guess that transmuting to a u32/u64 and checking it by hand would be too platform sensitive to be acceptable.)

I don't know, that was what I was wondering when I asked about assuming IEEE 754.

@catamorphism
Copy link
Contributor Author

Low, not 1.0

@thestinger
Copy link
Contributor

The FIXME mentioned in the issue is no longer there as these have been reimplemented. If there is a specific performance issue demonstrated, we can file a new issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. P-low Low priority
Projects
None yet
Development

No branches or pull requests

6 participants