[PATCH] D52144: use __ARM_FP instead of __VFP_FP__

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a reviewer: compnerd.
peter.smith added a comment.

I've checked with the gcc sources and it is __VFP_FP__ is unconditionally set by gcc. This might be a relatively recent change so I suspect that clang will have been following early behaviour of gcc. It will be worth updating the tests to use __ARM_FP as well. It will also be worth working out why the tests didn't fail for you as well. I'd only expect to see a difference in behaviour if you were using gcc without floating point support though.



================
Comment at: lib/builtins/CMakeLists.txt:571
       # For ARM archs, exclude any VFP builtins if VFP is not supported
+      # NOTE: __VFP_FP__ is unconditionally defined by gcc regardless of build
+      # flags, so it's not a good indicator to rely on.  Use __ARM_FP instead.
----------------
I suggest rewording the comment a bit. Once __VFP_FP__ has been removed then I don't think it is worth commenting about not using it. I think the review summary/source code commit message is the best place to mention that.

I think that your #TODO is correct. As it stands we can't compile compiler-rt if a cpu with a single precision only floating point unit is used. I note that the cmake recipe in clang/cmake/caches/BaremetalARM.cmake cheats and uses -mfpu=fp-armv8 which will allow the double precision files to compile. This is not ideal but the uses of double precision in assembly language seem to be restricted to Darwin, which I don't think are ever called from a single precision only target. It is possible in the future that architecturally illegal combinations are forbidden so this is something we should fix.

My suggestion for the comment is:
FIXME: We do not split out the single-precision and double-precision VFPv2 sources. This means that compiler-rt will not build if we specify a single precision only FPU. We may be able to use the value of __ARM_FP  to remove the double precision sources if double precision is not available.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52144