Update oclmath to add AArch64 support. #890
Conversation
|
CI is showing that the OpenCL CTS versions of these files are not clang-formatted. But if we clang-format them here, we do not avoid the problem that we needlessly have two different versions of these files. My thinking here is that two alternatives are preferable: we could exclude them from clang-formatting, leaving the responsibility with OpenCL CTS to keep it maintainable. Or we could apply the formatting to OpenCL CTS, if its style aligns sufficiently closely with SYCL CTS. Feedback would be welcome here. |
This replaces fpcontrol.h and rounding_mode.cpp/.h with the versions from OpenCL CTS. The old version included in SYCL-CTS did not support AArch64, and although it would be possible to just make the smallest possible change to get that to work, it seems like it will be easier for future changes for the OpenCL and SYCL versions of these files to be the same.
Feedback would still be welcome, but I have applied the clang-format changes so that CI should show as green, hopefully making it more likely to get someone to have a look. |
|
Can anyone remember me why we had these files in the CTS at the first place? |
@keryell, I think it was added to check the accuracy requirements for SYCL math functions in SYCL-1.2.1. @AerialMantis, did I get it right? |
|
I think we need someone with ARM expertise to review these changes. Considering that we borrowed this code from OpenCL-CTS, I think it's reasonable to ask @kpet to review this change. |
keryell
added
help wanted
|
@kpet nous avons besoin d'aide ! |
|
@keryell I'm on holiday at the moment but I've sent myself a reminder to look into this PR when I get back. |
Thanks! There is no emergency since unfortunately this PR is old, but it would be nice to have the feedback from a specialist. |
|
We've been using the OpenCL-CTS code on AArch64 targets for a good few years now and have not identified issues with it yet (that I know of!). I think this change can go in as is if the logic was ported from the OpenCL CTS. FWIW, I couldn't spot anything glaringly wrong after a very quick review. Long term we should think about getting support for low-level FP control into C libraries (or maybe C++). Any volunteers? :P |
This replaces fpcontrol.h and rounding_mode.cpp/.h with the versions from OpenCL CTS. The old version included in SYCL-CTS did not support AArch64, and although it would be possible to just make the smallest possible change to get that to work, it seems like it will be easier for future changes for the OpenCL and SYCL versions of these files to be the same.