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

std.math: use most inline x86/x64 assembly only if real is defined as x87 #3

Merged
merged 2 commits into from
Oct 21, 2014

Conversation

kinke
Copy link
Member

@kinke kinke commented Oct 18, 2014

Now that real == double on Win64. Useful in combination with ldc-developers/druntime#13 and MSVC x64 2013+.

{
version = INTRINSICS_FROM_303;
version = INTRINSICS_FROM_304;
}
Copy link
Member

Choose a reason for hiding this comment

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

version = INTRINSICS_FROM_305; is missing here.

@kinke
Copy link
Member Author

kinke commented Oct 20, 2014

No intrinsics introduced in LLVM 3.5/3.6 are used, that's why I limited the INTRINSICS_FROM_* to those which are actually required.

redstar added a commit that referenced this pull request Oct 21, 2014
std.math: use most inline x86/x64 assembly only if real is defined as x87
@redstar redstar merged commit 9108b11 into ldc-developers:ldc Oct 21, 2014
@redstar
Copy link
Member

redstar commented Oct 21, 2014

Does not build on Linux:

/home/travis/build/ldc-developers/ldc/runtime/phobos/std/math.d(90): Error: version INLINE_YL2X defined after use

See https://travis-ci.org/ldc-developers/ldc/jobs/38571952

@kinke
Copy link
Member Author

kinke commented Oct 21, 2014

Hmm, no idea what's causing this error. I've simply moved the version = INLINE_YL2X; statements into a static if (real.sizeof > double.sizeof) { ... } block.
Additionally, the problem seems to arise while compiling std.mathspecial, which publicly imports std.math. And according to Travis output, that phobos std.mathspecial seems to be compiled while building druntime (?!), specifically, rt.sections_linux and/or rt.sections_android...

@redstar
Copy link
Member

redstar commented Oct 21, 2014

The Travis output is not ordered because several processes are running in parallel.

@redstar
Copy link
Member

redstar commented Oct 21, 2014

I think the problem here is that the version blocks are evaluated first. This is done because the code of a not satisfied version is not compiled into the program. In contrast, the static if is always evaluated at compile time. The result is that version = INLINE_YL2X; is evaluated after all version guards have been evaluated. The TDPL rule is: You can set a version only before any read of that version.

It works in the Win64 case because the body of the static if is excluded.

@kinke
Copy link
Member Author

kinke commented Oct 21, 2014

Should be fixed in #4.

@dnadlinger
Copy link
Member

I don't think duplicating the INTRINSICS_FROM_… logic here is a good idea. Instead, use __traits(compiles, <… call to the intrinsic in question…>).

@redstar
Copy link
Member

redstar commented Oct 21, 2014

Good hint.

kinke pushed a commit to kinke/phobos that referenced this pull request Aug 6, 2017
Add documentation and unittests regarding multisets - take ldc-developers#3
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