Skip to content

Conversation

@thewilsonator
Copy link
Contributor

Add a bunch of overloads for intrinsics for in core.math.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@thewilsonator
Copy link
Contributor Author

Druntime & phobos parts to follow.

thewilsonator added a commit to thewilsonator/druntime that referenced this pull request Mar 10, 2018
@quickfur
Copy link
Member

Dumb question: is there a way to check that this emits the expected machine code? Or we simply don't have the facilities for that in place?

@wilzbach
Copy link
Contributor

Dumb question: is there a way to check that this emits the expected machine code? Or we simply don't have the facilities for that in place?

We do, it's just not very well-known and at the moment the it contains quite a lot of duplication:

https://github.com/dlang/dmd/blob/master/test/runnable/test_cdvecfill.d
https://github.com/dlang/dmd/blob/master/test/runnable/test_cdcmp.d
https://github.com/dlang/dmd/blob/master/test/runnable/test_cdstrpar.d

At some point this should probably be factored out into one small library and one update script ...

@wilzbach
Copy link
Contributor

@JackStouffer
Copy link
Contributor

What's the status of this? What still needs to be done in order to be considered?

@thewilsonator
Copy link
Contributor Author

I'd prefer #8002 to go instead of this but it fails for 32-bit, idk why, I won't have time to work on this until dconf. In the meantime this should suffice for the druntime and phobos parts assuming I have the mangling correct, but should probably be tested in conjunction with the associated druntime and phobos PRs to make sure.

@JinShil
Copy link
Contributor

JinShil commented Apr 15, 2018

This PR:

I'd prefer #8002 to go instead of this

#8002:

in the interest of moving forward with 18559 please review #7995

I'm in an infinite loop.

@thewilsonator
Copy link
Contributor Author

Haha.
This one works, is not the solution I'd prefer, but isn't necessarily bad (more of the same code) .
The other one is a better solution (removes use of strlen & friends and goes by symbol name rather then mangled) but doesn't pass 32-bit.

Go with this one, unless you want to figure out why the other one fails for 32-bit. I plan to get the other one working at dconf.

@WalterBright
Copy link
Member

Can we get this one pulled? #8071

@dlang-bot dlang-bot merged commit 50b81fa into dlang:master May 5, 2018
@thewilsonator thewilsonator deleted the fix18559 branch May 11, 2022 02:21
@ibuclaw
Copy link
Member

ibuclaw commented Jul 8, 2023

Possibly a regression introduced by this PR.

https://issues.dlang.org/show_bug.cgi?id=24040

There are float and double overloads for these core.math intrinsics, but they all still map to x87 instructions, and get treated as if the precision is 80-bit.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants