-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[NVPTX] libdevice support, enable NVPTX backend in topi tests #1365
Conversation
@tqchen ready for a review. can you have a look? |
const Call* call = e.as<Call>(); | ||
CHECK(call != nullptr); | ||
std::ostringstream intrinsic_name; | ||
intrinsic_name << "__nv_" << call->name << "f"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this works for double type? The f suffix seems to be hardcoded for float32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to dispatch by type to support double type
src/codegen/llvm/codegen_nvptx.cc
Outdated
if (fname.substr(0, 4) != "__nv") continue; | ||
// This is to strip off unused __nv_* functions from the final module | ||
// The one that is actually used will be inlined at call site | ||
// See Halide's LLVM_Runtime_Linker.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Halide's LLVM_Runtime_Linker.cpp -> adopted from Halide's runtime linker
. if you have referred to the code
src/codegen/llvm/codegen_nvptx.cc
Outdated
if (fname.substr(0, 4) != "__nv") continue; | ||
// This is to strip off unused __nv_* functions from the final module | ||
// The one that is actually used will be inlined at call site | ||
// Adopted from Halide's runtime linker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, adopted-> adapted, my mistake on the previous comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tqchen fixed lint and passed CI. Thanks for the review. |
Math function support has been missing for NVPTX backend. This PR attempts to fix the final missing piece.
All topi tests pass with nvptx backend.