Fix many type mismatches in the CUDA version of calc_digamma and calc_trigamma#25791
Fix many type mismatches in the CUDA version of calc_digamma and calc_trigamma#25791xuhdev wants to merge 6 commits intopytorch:masterfrom
Conversation
…_trigamma
- There are some missing casts.
- Functions like ::log, ::sin will potentially always invoke the double version on host. For
example, compiling the following code:
```c++
#include <cmath>
float log_float(float f) {
return ::logf(f);
}
double log_double(double f) {
return ::log(f);
}
float log_float2(float f) {
return ::log(f);
}
float log_float3(float f) {
return std::log(f);
}
```
using `g++ -c -O3` leads to:
log_float(float):
jmp logf
log_double(double):
jmp log
log_float2(float):
subq $8, %rsp
cvtss2sd %xmm0, %xmm0
call log
addq $8, %rsp
cvtsd2ss %xmm0, %xmm0
ret
log_float3(float):
jmp logf
Note that log_float2 delegates the call to the double version of log
(surrounded by cast), while log_float3 delegates the call correctly to
logf.
|
@pytorchbot rebase this please |
|
@pytorchbot rebase this please |
|
@pytorchbot rebase this please |
| } | ||
|
|
||
| bool x_is_integer = x == ::floor(x); | ||
| bool x_is_integer = x == std::floor(x); |
There was a problem hiding this comment.
I'm a little confused. Does gcc compile this, or does nvcc do it? The code ultimately gets run on device, right?
There was a problem hiding this comment.
The function is defined as __host__ __device__, that means it will be compiled twice on each
There was a problem hiding this comment.
@zou3519 The main point of this kind of change in this PR is about the confusion that, the CUDA doc is silent on this issue (given that std::floor exists), and that assuming ::floor is the same as std::floor is inconsistent with standard C++. I think it would best if we can be more compliant to avoid a potential issue, which by itself at least doesn't hurt.
There was a problem hiding this comment.
Okay, to make sure I understand what's going on: where is floor in ::floor coming from?
There was a problem hiding this comment.
It can come from a C definition (as declared in a C header file, which is solely defined for double). The standard C++ library is silent on ::floor, while it defines std::floor. Does this answer your question?
There was a problem hiding this comment.
That does, thanks for the clarification
|
@pytorchbot rebase this please |
|
@pytorchbot rebase this please |
|
@pytorchbot merge this please The CI failure is likely unrelated, because this PR does not change any GPU code, and the change of CPU code is very local. |
facebook-github-bot
left a comment
There was a problem hiding this comment.
@izdeby has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…_trigamma (#25791)
Summary:
- There are some missing casts.
- Functions like ::log, ::sin will potentially always invoke the double version on host. For
example, compiling the following code:
```c++
#include <cmath>
float log_float(float f) {
return ::logf(f);
}
double log_double(double f) {
return ::log(f);
}
float log_float2(float f) {
return ::log(f);
}
float log_float3(float f) {
return std::log(f);
}
```
using `g++ -c -O3` leads to:
log_float(float):
jmp logf
log_double(double):
jmp log
log_float2(float):
subq $8, %rsp
cvtss2sd %xmm0, %xmm0
call log
addq $8, %rsp
cvtsd2ss %xmm0, %xmm0
ret
log_float3(float):
jmp logf
Note that log_float2 delegates the call to the double version of log
(surrounded by cast), while log_float3 delegates the call correctly to
logf. See https://godbolt.org/z/KsRWwW
Pull Request resolved: pytorch/pytorch#25791
Differential Revision: D17452312
Pulled By: izdeby
fbshipit-source-id: 6276a011a373cd7cb144f9ecd84116aa206e7d1b
|
Unlanding this with #26444. The affected code does run on the GPU and likely broke the ROCm builds. std:: and :: is an important distinction on GPUs. See some comments in https://github.com/pytorch/pytorch/blob/master/aten/src/ATen/native/Distributions.h#L7. |
|
Oops, sorry about this; looks like I made the merge comment on the wrong thread. yes, it is causing ROCm failure. I'll look into this, as ROCm works with some std:: functions, while not all of them. |
…_trigamma (pytorch#25791) Summary: - There are some missing casts. - Functions like ::log, ::sin will potentially always invoke the double version on host. For example, compiling the following code: ```c++ #include <cmath> float log_float(float f) { return ::logf(f); } double log_double(double f) { return ::log(f); } float log_float2(float f) { return ::log(f); } float log_float3(float f) { return std::log(f); } ``` using `g++ -c -O3` leads to: log_float(float): jmp logf log_double(double): jmp log log_float2(float): subq $8, %rsp cvtss2sd %xmm0, %xmm0 call log addq $8, %rsp cvtsd2ss %xmm0, %xmm0 ret log_float3(float): jmp logf Note that log_float2 delegates the call to the double version of log (surrounded by cast), while log_float3 delegates the call correctly to logf. See https://godbolt.org/z/KsRWwW Pull Request resolved: pytorch#25791 Differential Revision: D17452312 Pulled By: izdeby fbshipit-source-id: 6276a011a373cd7cb144f9ecd84116aa206e7d1b
There are some missing casts.
Functions like ::log, ::sin will potentially always invoke the double version on host. For
example, compiling the following code:
using
g++ -c -O3leads to:Note that log_float2 delegates the call to the double version of log
(surrounded by cast), while log_float3 delegates the call correctly to
logf. See https://godbolt.org/z/KsRWwW