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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to