[PATCH] These builtin functions set errno. Mark them accordingly.

Richard Smith richard at metafoo.co.uk
Tue Dec 9 17:58:13 PST 2014


A few comments, then this seems OK with accompanying testcases. All the functions other than the ones I commented on are lowered directly to libm calls, so it's correct to mark them "e".

(Separately, I wonder why we lower these directly to libcalls rather than to intrinsic calls, and conversely why LLVM has intrinsics that are exactly equivalent to libm functions.)

================
Comment at: include/clang/Basic/Builtins.def:111-113
@@ -110,5 +110,5 @@
 BUILTIN(__builtin_fabsl, "LdLd", "ncF")
-BUILTIN(__builtin_fmod , "ddd"  , "Fnc")
-BUILTIN(__builtin_fmodf, "fff"  , "Fnc")
-BUILTIN(__builtin_fmodl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_fmod , "ddd"  , "Fne")
+BUILTIN(__builtin_fmodf, "fff"  , "Fne")
+BUILTIN(__builtin_fmodl, "LdLdLd", "Fne")
 BUILTIN(__builtin_frexp , "ddi*"  , "Fn")
----------------
This is wrong. We lower these to an `frem` instruction, which does not set `errno`.

================
Comment at: include/clang/Basic/Builtins.def:140-142
@@ -139,5 +139,5 @@
 BUILTIN(__builtin_powil, "LdLdi", "Fnc")
-BUILTIN(__builtin_pow , "ddd"  , "Fnc")
-BUILTIN(__builtin_powf, "fff"  , "Fnc")
-BUILTIN(__builtin_powl, "LdLdLd", "Fnc")
+BUILTIN(__builtin_pow , "ddd"  , "Fne")
+BUILTIN(__builtin_powf, "fff"  , "Fne")
+BUILTIN(__builtin_powl, "LdLdLd", "Fne")
 
----------------
Is this correct? We lower this to an intrinsic, and http://llvm.org/docs/LangRef.html#llvm-powi-intrinsic doesn't mention the possibility of the intrinsic setting `errno`.

http://reviews.llvm.org/D3806






More information about the cfe-commits mailing list