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 more FP-rounding LLVM intrinsics #4756

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

kinke
Copy link
Member

@kinke kinke commented Sep 20, 2024

Resolves #4754.

@kinke kinke force-pushed the more_round_intrinsics branch 4 times, most recently from 63cb01a to f5303a9 Compare September 20, 2024 15:55
@kinke
Copy link
Member Author

kinke commented Sep 22, 2024

Oof, once again buggy intrinsic semantics by the looks of it (definitely not 'does what the libm function would do, except for NOT setting errno'), at least when LLVM has to resort to x87 (32-bit x86 not defaulting to SSE etc.). The Win64 std.mathspecial failure is probably related to its 64-bit real.

@kinke
Copy link
Member Author

kinke commented Oct 6, 2024

Turns out the LLVM docs are wrong; there is a difference between llvm.lround.i64 and llvm.llround.i64 after all. In the -mtriple=x86_64-windows-msvc case, calling the lround vs. llround MSVCRT function. This fixed the 32-bit x86 issues too.

@kinke kinke marked this pull request as ready for review October 6, 2024 17:55
@JohanEngelen
Copy link
Member

Turns out the LLVM docs are wrong; there is a difference between llvm.lround.i64 and llvm.llround.i64 after all.

Out of interest, what exactly is wrong in the LLVM docs?

@kinke
Copy link
Member Author

kinke commented Oct 6, 2024

The syntax section uses llvm.lround.i64 exclusively.

@kinke kinke merged commit 495578b into ldc-developers:master Oct 7, 2024
20 checks passed
@kinke kinke deleted the more_round_intrinsics branch October 7, 2024 14:46
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.

Missing intrinsic lrint
2 participants