Add support for fpbuiltin accuracy lookup #9167
Conversation
|
@AlexeySachkov Can you review this PR? Some of the code in PR #8280 is taken from this PR. I've addressed the comment you made there. I would like to land this first to unblock progress in 8280. Thanks! |
| assert((Ty->isFloatTy() || Ty->isDoubleTy()) && | ||
| "Invalid type for FPAccuracy"); | ||
|
|
||
| // High and medium accuracy have the same requirement for all functions |
There was a problem hiding this comment.
Is there documentation for how these values are selected? May be a link here can be handy?
There was a problem hiding this comment.
For high, medium, and low, the accuracy requirements are copied from the icc compiler's -fimf-precision=[high|medium|low] option (https://www.intel.com/content/www/us/en/docs/dpcpp-cpp-compiler/developer-guide-reference/2023-0/fimf-precision-qimf-precision.html). To those who have never used icc, these values are essentially arbitrary, but we've found them to be good choices for users.
I don't intend for this to be permanently bound to the icc meaning, so I don't think this is worth mentioning in the comments.
llvm/include/llvm/IR/FPAccuracy.def
Outdated
| // Note: for single-precision fdiv and sqrt, the value returned here assumes | ||
| // that the "-cl-fp32-correctly-rounded-divide-sqrt " option is not used. |
There was a problem hiding this comment.
`
| // Note: for single-precision fdiv and sqrt, the value returned here assumes | |
| // that the "-cl-fp32-correctly-rounded-divide-sqrt " option is not used. | |
| // Note: for single-precision fdiv and sqrt, the value returned here assumes | |
| // that the "-cl-fp32-correctly-rounded-divide-sqrt" option is not used. |
-cl-fp32-correctly-rounded-divide-sqrt is OpenCL flag, but this table defines accuracy only for SYCL and CUDA. This note is confusing.
Are there plans to define accuracy for other GPU programming languages like HIP, OpenMP offloading, OpenCL?
There was a problem hiding this comment.
I've updated the comment. I'm not aware of any current plans to define accuracy levels for other models. I'm not sure what requirements other platforms have. In fact, I think even the CUDA accuracy is a statement of the actual accuracy provided by CUDA's math library and not a "requirement" as such. I think its usefulness here is for people who aren't using CUDA but want results that are as accurate as they would have gotten with CUDA. The SYCL level defined here is actually derived from the OpenCL standard, and it would also apply to OpenMP implementations that use OpenCL as their offload mechanism. I couldn't find any accuracy requirement documentation for HIP.
This patch adds a new interface that can be used to lookup the required accuracy of floating-point builtin intrinsics for a specified type and accuracy level (high, medium, low, sycl, or cuda). A later change will add support for this functionality in the IRBuilder class.
|
The #include in the LIT test seems to still be there. When removed, this LGTM. Thanks. |
This patch adds a new interface that can be used to lookup the required accuracy of floating-point builtin intrinsics for a specified type and accuracy level (high, medium, low, sycl, or cuda).
A later change will add support for this functionality in the IRBuilder class.