device-libs: Move special case check in rsqrt f64 implementation #821
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
amd/device-libs/ocml/src/rsqrtD.cl
Outdated
| { | ||
| double y0 = BUILTIN_AMDGPU_RSQRT_F64(x); | ||
| double e = MATH_MAD(-x*y0, y0, 1.0); | ||
| double scale = x == PINF_F64 || x == 0.0 ? y0 : x; |
There was a problem hiding this comment.
No need to give this a name. Just factor it into the first argument to MAD.
amd/device-libs/ocml/src/rsqrtD.cl
Outdated
| double scale = x == PINF_F64 || x == 0.0 ? y0 : x; | ||
|
|
||
| double e = MATH_MAD(-scale * y0, y0, 1.0); | ||
| double y1 = MATH_MAD(y0*e, MATH_MAD(e, 0.375, 0.5), y0); |
There was a problem hiding this comment.
No need to name the return value now; we can just return the expression.
arsenm
force-pushed
the
device-libs/move-special-case-check-f64-rsqrt
branch
from
fd1fd34 to
b47ab92
Compare
arsenm
force-pushed
the
device-libs/move-special-case-check-f64-rsqrt
branch
from
b47ab92 to
9e9cc10
Compare
Move the edge case check to be on the original input argument, instead of the output of the rsq. This simplifies optimizations to strip out the check based on value tracking. This approximately results in equivalently good code. On targets with v_fmac_f64, the result looks worse in the most trivial example due to a bad decision to not rewrite to v_fma_f64 on the final fma (llvm#171891). The result is equivalently good in other final use contexts.
arsenm
force-pushed
the
device-libs/move-special-case-check-f64-rsqrt
branch
from
9e9cc10 to
2d3bf20
Compare
Move the edge case check to be on the original input argument, instead of the output of the rsq. This simplifies optimizations to strip out the check based on value tracking. This approximately results in equivalently good code. On targets with v_fmac_f64, the result looks worse in the most trivial example due to a bad decision to not rewrite to v_fma_f64 on the final fma (llvm#171891). The result is equivalently good in other final use contexts.