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

[WIP] Set real == double on Win64 MSVC. #747

Merged
merged 1 commit into from
Oct 16, 2014

Conversation

redstar
Copy link
Member

@redstar redstar commented Oct 12, 2014

The MS C Runtime does not support 80bit reals. Drop support in D, too.

@redstar
Copy link
Member Author

redstar commented Oct 12, 2014

@Trass3r @kinke Maybe you want to try this version?

@kinke
Copy link
Member

kinke commented Oct 12, 2014

Thanks Kai, that's most likely a sane decision for MSVC and may as well increase math performance (Phobos' std.math seems to encourage the use of reals, doesn't it?). Will try when I can - I'm currently using a core.stdc.math backported to LDC from dlang/druntime#968, for which I had to comment out ldc.longdouble to get rid of ambiguous symbols when linking against MSVC 2013.

@Trass3r
Copy link
Contributor

Trass3r commented Oct 13, 2014

That whole real by default attitude in phobos is just wrong anyway, esp. for x64 and non-x86.
I use stdc.tgmath though I wish calls like sin,exp,... would automatically be turned into llvm intrinsics.

@kinke
Copy link
Member

kinke commented Oct 13, 2014

+1 to that! Even worse, std.math implicitly assumes real to be 80-bit for x64 (version(D_InlineAsm_X86_64) guarding inline assembly). Imho, std.math should rely on core.stdc.math for the float and double overloads instead of forwarding to the phobos real implementation; almost all supported C runtimes provide all required functions anyway (even MS since 2013).
I'm not sure as to where we should place the LLVM intrinsics, i.e., std.math exclusively (where they currently are, at least those for reals) or in both core.stdc.math and std.math (only for the real implementations, the float/double functions would be aliases to core.stdc.math, as in core.stdc.tgmath). Yeah well, probably in both places, I guess.

@Trass3r
Copy link
Contributor

Trass3r commented Oct 13, 2014

Note that there's also the crippled core.math.
The reason for std.math reimplementing functions was given as the C versions being unreliable or something like that. Still real shouldn't be the default let alone forced on users like it currently is. But I think Walter needs to be convinced first IIRC.

@redstar
Copy link
Member Author

redstar commented Oct 13, 2014

This version caused an assertion failure in std.math. I have to fix this before I can merge this version.
If we are going this route (and IMHO it is wrong to fight against the C runtime) then we have to cleanup the math stuff.
BTW: On Linux/PPC I dropped the support for doubledouble type, too. It worked very well.

@redstar redstar force-pushed the msvcdouble branch 3 times, most recently from 55a04e9 to 9378624 Compare October 16, 2014 04:39
The MS C Runtime does not support 80bit reals. Drop support in D, too.
@redstar
Copy link
Member Author

redstar commented Oct 16, 2014

I found the root cause of the assertion failure. If the Travis build goes green then I merge this pull request.

redstar added a commit that referenced this pull request Oct 16, 2014
Set real == double on Win64 MSVC.
@redstar redstar merged commit 26446a2 into ldc-developers:master Oct 16, 2014
@redstar redstar deleted the msvcdouble branch October 16, 2014 16:35
@Trass3r
Copy link
Contributor

Trass3r commented Oct 24, 2014

Potential regression: #761

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.

3 participants