[PATCH] D12633: Implement ACLE 2.0 macros of chapters 6.6 and 6.7 for [ARM] and [Aarch64] targets

[Richard Barton via cfe-commits]

richard.barton.arm added a comment.

Hi all

I am uncomfortable with this patch for a number of reasons. These macros seem to me to be defined by the ACLE as describing the behaviour of the combination of library and compiler. For example, the __STDC_IEC_599__ macro would need some standards compliance from the compiler in terms of optimisations on floating point functions, and from the libraries in terms of the behaviour of the fp functions there. Thus, I don't think it is safe for clang to define this macro for all ARM compilation regardless of which library is being used to link with. Perhaps I have misunderstood that one - so it would be worth running this past Tim directly as the __ARM_FP_FENV_ROUNDING definition comes from his original AArch64 patches from early 2013.

If I am right - then clang would want to adopt an approach similar to GCC's described here https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html (grep for __GCC_IEC_559) where the compiler sets a macro which presumably is read by a compliant c library header which then sets the ACLE macro.

However, there are multiple bug reports in the area of FP math - see https://llvm.org/bugs/show_bug.cgi?id=8100 - which makes me think that for Clang to really be ACLE compliant, we should not be defining any of these macros apart from __ARM_FP_FAST. Additionally a bit of research on the devs list suggest that LLVM does not support signaling NaNs e.g. http://lists.llvm.org/pipermail/llvm-dev/2014-April/071699.html and http://lists.llvm.org/pipermail/llvm-dev/2014-August/075749.html from last year

I suggest that the patch needs to be resubmitted without the setting of __ARM_FP_FENV_ROUNDING, __STDC_IEC_559 and __SUPPORT_SNAN, and that the setting of __ARM_FP_FENV_ROUNDING in the aarch64 target be clarified.


http://reviews.llvm.org/D12633