This is an archive of the discontinued LLVM Phabricator instance.

[CUDA] Implemented device-side support for functions in <cmath>.
ClosedPublic

Authored by tra on Jan 26 2016, 11:32 AM.

Details

Summary

CUDA expects math functions in std:: namespace to work on device side.
In order to make it work with clang without allowing device-side code
generation for functions w/o appropriate target attributes, this patch
provides device-side implementations for <cmath> functions. Most of
them call global-scope math functions provided by CUDA headers. In few
cases we use clang builtins.

Tested out-of tree by compiling and running thrust's unit_tests.
https://github.com/thrust/thrust/tree/master/testing

Diff Detail

Repository
rL LLVM

Event Timeline

tra updated this revision to Diff 46019.Jan 26 2016, 11:32 AM
tra retitled this revision from to [CUDA] Implemented device-side support for functions in <cmath>. .
tra updated this object.
tra added a reviewer: jlebar.
tra added a subscriber: cfe-commits.
jlebar edited edge metadata.Jan 26 2016, 11:57 AM

Missing (?) functions:

  • div, ldiv, lldiv, imaxdiv
  • imaxabs

If you left these out intentionally (I forget if nvidia supports div_t), that's probably fine, but maybe add a comment?

wrt the "::" comments, some are nits because I think we end up calling the right thing, assuming that nobody defines the wrong thing in namespace std. But some aren't nits because afaict they lead to infinite recursion. We should probably just give fully-qualified names for all functions we call.

lib/Headers/__clang_cuda_cmath.h
33 ↗(On Diff #46019)

Why is std::abs a special case that needs to be called out here?

38 ↗(On Diff #46019)

We should probably respect the standard and not define functions that aren't available in C++11 if we're not in c++11 mode.

93 ↗(On Diff #46019)

Looking through bugzilla, it appears that this builtin may try to invoke library functions, which may or may not exist in our case (e.g. extern "C" fabs vs ::fabs). It's probably worth checking this one in particular to make sure it works.

99 ↗(On Diff #46019)

Nit, ::

100 ↗(On Diff #46019)

Need :: to avoid infinite recursion, right?

105 ↗(On Diff #46019)

Where's finitef coming from? Same for the other functions used here.

105 ↗(On Diff #46019)

Nit: "::" in front of all the __ functions.

146 ↗(On Diff #46019)

Nit: ::ldexpf?

147 ↗(On Diff #46019)

:: not optional on this one, right?

tra updated this revision to Diff 46041.Jan 26 2016, 1:34 PM
tra edited edge metadata.
tra marked 3 inline comments as done.

Added missing ::

tra updated this revision to Diff 46055.Jan 26 2016, 2:39 PM
tra marked 6 inline comments as done.

Fixed few issues revealed by -Wdouble-promotion

tra added inline comments.Jan 26 2016, 2:42 PM
lib/Headers/__clang_cuda_cmath.h
39 ↗(On Diff #46041)

Both libc++ and libstdc++ provide these symbols in pre-c++11 mode, so should we.

33 ↗(On Diff #46019)

It's a bit of an oddball because it operated on integer parameters and came from <cstdlib>.
Which is indeed irrelevant here.
Deleted.

93 ↗(On Diff #46019)

Checked -- as far as I can tell it generates valid PTX to test its arguments.

99 ↗(On Diff #46019)

Fixed.

105 ↗(On Diff #46019)

finitef and other non-builtin * functions come from CUDA headers. Added missing ::.

jlebar accepted this revision.Jan 26 2016, 2:44 PM
jlebar edited edge metadata.
This revision is now accepted and ready to land.Jan 26 2016, 2:44 PM
This revision was automatically updated to reflect the committed changes.